3 Replies - 870 Views - Last Post: 29 October 2012 - 03:32 PM Rate Topic: -----

#1 Duta  Icon User is offline

  • New D.I.C Head

Reputation: 3
  • View blog
  • Posts: 43
  • Joined: 21-June 12

[Lua] Is my code good Lua style?

Posted 29 October 2012 - 10:32 AM

So I just wrote my first program in Lua and was wondering if anyone with knowledge of the language check if over for good programming style; as I'm new to the language I don't know if my way is considered 'correct'.
--Character class
Character = {}
Character.__index = Character
function Character:new()
	local self = {}
	setmetatable(self, Character)
	self.name  = "Unknown"
	self.level = "Unknown"
	self.class = "Unknown"
	self.race  = "Unknown"
	return self
end
function Character:load(name, level, class, race)
	if name  ~= nil then self.name  = name  end
	if level ~= nil then self.level = level end
	if class ~= nil then self.class = class end
	if race  ~= nil then self.race  = race  end
	return self
end

--Util functions
Util = {}
function Util:concat(aString, length)
	local blanks = string.rep(" ", length)
	local s = aString..blanks
	return string.sub(s, 0, length)
end
function Util:printtable(items, vars)
	local div = "|"
	for i, var in ipairs(vars) do
		local maxlength = 0
		for i, item in ipairs(items) do
			local len = string.len(item[var.varname])
			if len > maxlength then
				maxlength = len
			end
		end
		div = div..string.rep("-", maxlength).."|"
		var.maxlength = maxlength
	end
	print(div)
	local totlen = 0
	local numvars = 0
	io.write("|")
	for i, var in ipairs(vars) do
		io.write(Util:concat(var.name, var.maxlength).."|")
		totlen = totlen + var.maxlength
		numvars = numvars + 1
	end
	print()
	print(div)
	for i, item in ipairs(items) do
		io.write("|")
		for i, var in ipairs(vars) do
			io.write(Util:concat(item[var.varname], var.maxlength).."|")
		end
		print()
	end
	print(div)
end

--Var class
Var = {}
Var.__index = Var
function Var:new()
	local self = {}
	setmetatable(self, Var)
	return self;
end
function Var:load(varname, name)
	self.varname = varname
	self.name = name
	return self
end

--Test chars
local chars = {
	Character:new():load("Painrc", 25,  "Rogue", "Orc"  ),
	Character:new():load("Geoff",  90,  "Mage",  "Gnome"),
	Character:new():load("Anon",   nil, "/b/",   "4chan")
}

--Print test chars
Util:printtable(chars, {
	Var:new():load("name",  "Name" ),
	Var:new():load("level", "Level"),
	Var:new():load("class", "Class"),
	Var:new():load("race",  "Race" )
})
for i,char in ipairs(chars) do
	print(char.name, char.isHorde())
end


This post has been edited by Duta: 29 October 2012 - 10:43 AM


Is This A Good Question/Topic? 0
  • +

Replies To: [Lua] Is my code good Lua style?

#2 ishkabible  Icon User is offline

  • spelling expret
  • member icon




Reputation: 1622
  • View blog
  • Posts: 5,709
  • Joined: 03-August 09

Re: [Lua] Is my code good Lua style?

Posted 29 October 2012 - 10:49 AM

--Character class
Character = {}
Character.__index = Character
function Character:new()
	local self = {}
	setmetatable(self, Character)
	self.name  = "Unknown"
	self.level = "Unknown"
	self.class = "Unknown"
	self.race  = "Unknown"
	return self
end


'nil' is generally a better value for saying that the value is unknown, that's what it's there for.
function Character:load(name, level, class, race)
	if name  ~= nil then self.name  = name  end
	if level ~= nil then self.level = level end
	if class ~= nil then self.class = class end
	if race  ~= nil then self.race  = race  end
	return self
end


the 'if' statements have no effect if you us 'nil' for the unknown value in the first place; hence 'nil' works out a bit better.
function Character:isHorde()

   if ( self.race == "Human"        or
        self.race == "Night Elf"    or
        self.race == "Dwarf"        or
        self.race == "Gnome"        or
        self.race == "Draenei"
      )
   then return false;
   else return true;
   end
end


this is ok, I might use a 'is element of function' with a table containing all those races but that's just me.
--Util functions
Util = {}
function Util:concat(aString, length)
	local blanks = string.rep(" ", length)
	local s = aString..blanks
	return string.sub(s, 0, length)
end


I don't see where 'self' is used here yet you declare it ':' not '.'. also you can use __concat function in your objects metatable so that the '..' operator can be used.
function Util:printtable(items, vars)
	local div = "|"
	for i, var in ipairs(vars) do
		local maxlength = 0
		for i, item in ipairs(items) do
			local len = string.len(item[var.varname])
			if len > maxlength then
				maxlength = len
			end
		end
		div = div..string.rep("-", maxlength).."|"
		var.maxlength = maxlength
	end
	print(div)
	local totlen = 0
	local numvars = 0
	io.write("|")
	for i, var in ipairs(vars) do
		io.write(Util:concat(var.name, var.maxlength).."|")
		totlen = totlen + var.maxlength
		numvars = numvars + 1
	end
	print()
	print(div)
	for i, item in ipairs(items) do
		io.write("|")
		for i, var in ipairs(vars) do
			io.write(Util:concat(item[var.varname], var.maxlength).."|")
		end
		print()
	end
	print(div)
end


Again, I don't see where 'self' is used here. and all those 'concat' calls could be just a simple '..' operator
--Var class
Var = {}
Var.__index = Var
function Var:new()
	local self = {}
	setmetatable(self, Var)
	return self;
end


this is fine
function Var:load(varname, name)
	self.varname = varname
	self.name = name
	return self
end


fine I suppose
--Test chars
local chars = {
	Character:new():load("Painrc", 25,  "Rogue", "Orc"  ),
	Character:new():load("Geoff",  90,  "Mage",  "Gnome"),
	Character:new():load("Anon",   nil, "/b/",   "4chan")
}

--Print test chars
Util:printtable(chars, {
	Var:new():load("name",  "Name" ),
	Var:new():load("level", "Level"),
	Var:new():load("class", "Class"),
	Var:new():load("race",  "Race" )
})


rather than Var:new():load("race", "Race" ) by adding those arguments to your 'new' constructor you could just make it Var.new("race", "Race");. same with Character. even cooler you could overload the call operator for 'Var' or 'Character' so that it was just Var("race", "Race"); or if you were feeling fancy check for a table and use it
Var{"race", "Race"}


spark notes:
  • with some clever use of operator overloading and nil you can reduce your code size by a good bit
  • use '.' if a table just acts as a namespace or static function ':' if it's used with an object. ':' passes an unused argument otherwise
  • you are introducing unnecessary calls, namely with 'load'

This post has been edited by ishkabible: 29 October 2012 - 11:05 AM

Was This Post Helpful? 0
  • +
  • -

#3 Duta  Icon User is offline

  • New D.I.C Head

Reputation: 3
  • View blog
  • Posts: 43
  • Joined: 21-June 12

Re: [Lua] Is my code good Lua style?

Posted 29 October 2012 - 11:54 AM

Thanks ishka, that was really useful :)
I tried changing Util:concat and Util:printtable to Util.concat and Util.printtable (in the function declarations and the uses), however doing that gives me the output:
||||
||||
||||
||||

Whereas normally I get:
|------|-------|-----|-----|
|Name  |Level  |Class|Race |
|------|-------|-----|-----|
|Painrc|25     |Rogue|Orc  |
|Geoff |90     |Mage |Gnome|
|Anon  |Unknown|/b/  |4chan|
|------|-------|-----|-----|

Any ideas?
Also, I didn't mean to include the Character:isHorde function as I don't use it, but good point. I wasn't happy with it as it stood :)
Finally, how would you overload the call operator?
Was This Post Helpful? 0
  • +
  • -

#4 ishkabible  Icon User is offline

  • spelling expret
  • member icon




Reputation: 1622
  • View blog
  • Posts: 5,709
  • Joined: 03-August 09

Re: [Lua] Is my code good Lua style?

Posted 29 October 2012 - 03:32 PM

did you change it every where? if you change the definition but not the calls it will pass an extra argument.

if you define a function with ':' it adds an implicit self parameter
function x:y() end

is the same as
function x.y(self) end

which is also the same as
x.y = function(self) end

which is short hand further for
x["y"] = function(self) end


when you call a function like so
x.y()

it's the same as
(x["y"])()

or
f = x["y"]
f()


when you use ':' however it's different.
x:y(1, 2, 3)

is the same as...
x.y(x, 1, 2, 3)

which again is the same as
(x["y"])(x, 1, 2, 3)

Was This Post Helpful? 0
  • +
  • -

Page 1 of 1