14 replies to this topic

[Lupus590]

Maybe not so solved...

http://www.computerc...post__p__239950

old post

So I'm working on an update to my quadPipe program and am having a problem interpreting the command line arguments, I have a for loop and am looping through everything in (admittedly) a very bad way. I'm considering replacing the for loop with a while loop, but want to pick your brains first.

If a rewrite is needed then consider this a "how not to write a for loop".

Full Code

Relevant Code

local arg = {...}
--set vaiables to default, we will overide these with the arg later
local quadProgram = defaults.quadProgram
local numOfQuads = defaults.numOfQuads
local delayMult = defaults.delayMult
local ignoreRedstone = defaults.ignoreRedstone
local anyKeyTerminate = defaults.anyKeyTerminate
local bSide = defaults.bSide
local rSide = defaults.rSide

--lower case everything
for i = 1, #arg do
  if tostring(arg[i]) then
	arg[i] = string.lower(arg[i])
  end
end
--process arg
for i = 1, #arg do
  if arg[i] == "-h" then
	printUsage()
  elseif arg[i] == "-f" then --quadProgram
	do --make my editor look clearer
	myError("arg "..i.." Can't load file", 2)
	i = i + 1 --this is definatly really bad practise
	end --make my editor look clearer
  elseif arg[i] == "-n" then --numOfQuads
	do --make my editor look clearer
	local n = tonumber(arg[i+1])
	if n == nil or n < 1 then
	  myError("arg "..i.." -n arg must be followed by a whole number greater than 0 (1 or greater are valid)", 2)
	else
	  local a,b = math.modf(n)
	  if b ~= 0 then
		myError("arg "..i.." -n arg must be followed by a whole number greater than 0 (1 or greater are valid)", 2)
	  end
	end
	if n > 64 then
	  print("arg "..i.." Warning: numOfQuads is larger than one stack(64). Did the max per base change or are you piping them in?")
	end
	numOfQuads = n
	i = i + 1 --this is definatly really bad practise
	end --make my editor look clearer
  elseif arg[i] == "-d" then --delayMult
	do --make my editor look clearer
	local d = tonumber(arg[i+1])
	if d == nil then
	  myError("arg "..i.." -d arg must be followed by a number", 2)
	end
	delayMult = d
	i = i + 1 --this is definatly really bad practise
	end --make my editor look clearer
  elseif arg[i] == "-r" then --redstoneSide
	do --make my editor look clearer
	local r = tostring(arg[i+1])
	if r == nil then
	 myError("arg "..i.." -r arg must be followed by a string representing a side or equal to 'ignore'", 2)
	end
	if r == "ignore" then
	  ignoreRedstone = true
	elseif r == "left" or r == "right" or r == "front" or r == "back" then
	  rSide = r
	  ignoreRedstone  = false
	else
	  myError("arg "..i.." -r arg must be followed by a string representing a side or equal to 'ignore'", 2)
	end
	i = i + 1 --this is definatly really bad practise
	end --make my editor look clearer
  elseif arg[i] == "-b" then --baseSide
	do --make my editor look clearer
	local b = tostring(arg[i+1])
	if b == nil then
	  myError("arg "..i.." -b arg must be followed by a string representing a side", 2)
	end
	if b == "left" or b == "right" or b == "front" or b == "back" then
	  bSide = r
	else
	  myError("arg "..i.." -b arg must be followed by a string representing a side", 2)
	end
	i = i + 1 --this is definatly really bad practise
	end --make my editor look clearer
  elseif arg[i] == "-t" then --anyKeyTerminate
	do --make my editor look clearer
	local t = tostring(arg[i+1])
	if t == nil then
	  nyError("arg "..i.." -t arg must be followed by a string equal to 'true' or 'false'", 2)
	end
	if t == "true" then
	  anyKeyTerminate = true
	elseif t == "false" then
	  anyKeyTerminate = false
	else
	  myError("arg "..i.." -t arg must be followed by a string equal to 'true' or 'false'", 2)
	end
	i = i + 1 --this is definatly really bad practise
	end --make my editor look clearer
  else
	myError("arg "..i.." unprocessed arg",2)
  end
end

Edited by Lupus590, 09 December 2015 - 03:07 PM.

[Dog]

If I could request a little clarification - what problem are you having interpreting the command line arguments? Are some being skipped? None working?

I noticed a couple of things you may want to take another look at:

- Not sure if you care about this but, unless I'm mistaken, your 'lower case everything' loop will crash if one of the arguments provided is a number

- This may be part of your problem ... regarding your use of i = i + 1 in your 'process arg' loop - I'm not exactly sure what you're trying to do, but based on what you've posted, I'm guessing that you're trying to increment the value of 'i' used by the for loop. I may be wrong about this (too), but I don't think that actually modifies the value of i that is defined by the loop. I tried something similar in the past and eventually came to the conclusion that the variable used for the loop can't be 'adjusted' except by the loop itself. Unless I'm mistaken, you're actually creating another variable named i, within the loop, separate from the variable that is holding the current value for the loop. In your particular case, I think this would work...

for i = 1, #arg, 2 do

since you're parsing every other variable and using arg[i+1] to get the ones in between.

Edited by Dog, 22 November 2015 - 09:50 PM.

[Lupus590]

your code snippet will not work at -h is a single argument

as for the actual problem, my unprocessed arg error goes off (complaining about the second arg) when I pass these as arguments

 quadPipe -t true 

to me that means that

 i = i+1 

doesn't work

the lowercase everything loop checks if the arg is a string before lowering, not tried it yet on numbers though, I can always add an escape condition

while loop with my own loop counter will have to do then, as messy as that will end up being


To clarify I'm replacing the problem for loop with a while loop, it may be a bit more messy but it should work.

Edited by Lupus590, 22 November 2015 - 10:34 PM.

[Dog]

Lupus590, on 22 November 2015 - 10:12 PM, said:

your code snippet will not work at -h is a single argument
...
the lowercase everything loop checks if the arg is a string before lowering, not tried it yet on numbers though

Uggg - I missed the -h - apologies for a useless suggestion.

I may be incorrect (again), but I believe your lowercase everything loop isn't actually checking if arg[i] is a string, it's just applying tostring to arg[i] without capturing the output. You can successfully tostring(nil) as far as I know - which would pass your if tostring(arg[i]) check if I'm not mistaken. To check if it's a string, I think you probably want/need to use something like this...

if type(arg[i]) == "string" then

Edited by Dog, 22 November 2015 - 10:34 PM.

[KingofGamesYami]

Since it's a command line argument, it will always be a string. So checking whether or not you have a string is redundant.

Why not use patterns and string manipulation to make life easy?

local args = {...}

for a, b in table.concat( args, " " ):gmatch( "%-(%a)%s(%a+)" ) do
  ...
end

In your example

quadPipe -t true

This would make a equal to "t", and b equal to "true".

Edited by KingofGamesYami, 22 November 2015 - 10:39 PM.

[Lupus590]

I've replaced it with a while loop now, I'll check if that fixed it

[Dog]

KingofGamesYami, on 22 November 2015 - 10:36 PM, said:

Since it's a command line argument, it will always be a string. So checking whether or not you have a string is redundant.
...

That makes perfect sense, thank you.

Apologies for the bad advice, Lupus590 :/

Edited by Dog, 22 November 2015 - 10:42 PM.

[Lupus590]

Well, it's stopped crashing... which means that the problem is harder to trace.

I'll bash my head on the keyboard a bit, and maybe sleep on it.

The new bug is that any arg is valid now and they don't change the variables/trigger the if statements (but do the while loop).

new code (under new paste)

Edited by Lupus590, 22 November 2015 - 10:51 PM.

[Dog]

I did notice a couple of things:

On line 125 you nyError instead of myError.

Second thing - if you only pass a single argument (e.g. -h), this

while i < #arg do

will skip the loop if #arg == 1 since 1 isn't less than 1. I think you want...

while i <= #arg do

Edited by Dog, 22 November 2015 - 11:14 PM.

[H4X0RZ]

You could also use my (old) code I made exactly for this Kind of work.

http://pastebin.com/zb5JMzbb

local cli = dofile "path to cli"
local args = cli.new{...}
args:setDefault("someValue",true)
args:parse()

--now handle the data
if not args:getBoolean "someValue" then 
  --do stuff
end

[Lupus590]

Dog, on 22 November 2015 - 11:10 PM, said:

I did notice a couple of things:

On line 125 you nyError instead of myError.

Second thing - if you only pass a single argument (e.g. -h), this

while i < #arg do

will skip the loop if #arg == 1 since 1 isn't less than 1. I think you want...

while i <= #arg do

lets hope that those are the last of the bugs.

H4X0RZ, on 23 November 2015 - 12:33 AM, said:

You could also use my (old) code I made exactly for this Kind of work.

http://pastebin.com/zb5JMzbb

local cli = dofile "path to cli"
local args = cli.new{...}
args:setDefault("someValue",true)
args:parse()

--now handle the data
if not args:getBoolean "someValue" then
  --do stuff
end

I do remember finding your post for that, but I couldn't find it again when I wanted to use it (have now, got it bookmarked in a more logical place now)
I've gotten this far without it, so I'd like to continue. It's part of the fun.

[Lupus590]

I think that everything is working now (except the file part, but that's because I haven't written that yet.)

Thanks for all the help guys!

Fixed version of the code (file loading still WIP)

Edited by Lupus590, 23 November 2015 - 01:26 PM.

[Lupus590]

You may have noticed a lack of an update on the quadPipe thread, I've had another bug and (until recently) not had the time/energy to look into it.

the test file is this (and is named test)

{"up","down"}

the current code: http://pastebin.com/mkN2XJRJ

the error: quadpipe:63: attempt to index ? (a nil value)

what I typed to get the error: quadpipe -f test

Edited by Lupus590, 09 December 2015 - 01:12 PM.

[Wojbie]

You appear to be using "args" instead of "arg" in few lines.

[Lupus590]

that would do it

all bugs fixed, time for cleanup and more testing

Edited by Lupus590, 09 December 2015 - 03:06 PM.