Problems with string and variables (exporting program)

Introduction

I am having an issue with a simple program I am making to make mass producing of programs on floppies and such, more efficient and cleaner.
The issue comes (I assume) when I try to define read() as a word and not a string, and then try to put that into a command which requires strings.
I want the program to essentially ask for the drive side, the file, move it to the disk in that drive, and then rename the disk to whatever the user pleases. I haven't done the last part yet. I want to fix this part first.

The Code

selDrive = nil
term.clear()
term.setCursorPos(1,1)
term.setTextColour(colours.yellow)
term.write("What side is the drive with the disk?")
term.setCursorPos(2,5)
term.setTextColour(colours.white)
term.write("Examples: right, front, back, top")
term.setCursorPos(2,3)
term.setTextColour(colours.blue)

while true do
  response1 = read()
  term.setCursorPos(2,3)
  term.clearLine()

  if response1 == "top"
	then selDrive = top
	break
  elseif response1 == "bottom"
	then selDrive = bottom
	break
  elseif response1 == "front"
	then selDrive = front
	break
  elseif response1 == "back"
	then selDrive = back
	break

  elseif response1 == "left"
	then selDrive = left
	break
  elseif response1 == "right"
	then selDrive = right
	break
end
end

term.clear()
term.setCursorPos(1,1)
term.setTextColour(colours.yellow)
term.write("What would you like to export?")
term.setCursorPos(2,5)
term.setTextColour(colours.white)
print("The name of the file; for example: adventure, hello, help")
term.setCursorPos(2,3)
term.setTextColour(colours.blue)

file = read()
term.clear()

dir = disk.getMountPath(selDrive)

fs.copy(file,dir)
sleep(1)

Try something like this:

local selDrive = nil
-- Your term clearing and setup
while true do
  local response1 = read()
  -- More term clearing
  if response1 == "top" then
    selDrive = "top"
    break
  ...

Notice how "top" is meant to be as a string. What your program is erroring out on is it thinks "Oh, he wants to make selDrive the same as top, let me just find it…wait, there is no top…whut?" and crashes. Without the quote marks it treats it as a variable, and since "top" was never initialised it thinks it's nil (uncalled), erroring. Use strings. Do the same for all the others as well (bottom, front etc.).

Use local variables (like I did in my code) as well.

What's wrong with setting selDrive = response1 (or replacing all instances of selDrive with response1)?

If you want to validate the user's input, try disk.hasData(response1). If it returns true, that means that the disk drive exists and contains a floppy disk.

The Crazy Phoenix, on 10 August 2017 - 08:02 AM said:

What's wrong with setting selDrive = response1 (or replacing all instances of selDrive with response1)?

If you want to validate the user's input, try disk.hasData(response1). If it returns true, that means that the disk drive exists and contains a floppy disk.

Dave-ee Jones, on 10 August 2017 - 07:16 AM said:

-snip-

Notice how "top" is meant to be as a string. What your program is erroring out on is it thinks "Oh, he wants to make selDrive the same as top, let me just find it…wait, there is no top…whut?" and crashes. Without the quote marks it treats it as a variable, and since "top" was never initialised it thinks it's nil (uncalled), erroring. Use strings. Do the same for all the others as well (bottom, front etc.).

Use local variables (like I did in my code) as well.

To elaborate on these…
When you write `selDrive = top`, Lua's looking for a variable called `top`, and finding nothing, so this is equivalent to `selDrive = nil`, which clearly isn't what you want. To fix this, you can put parenthesis around `top`: `selDrive = "top".
Having done this, you'll notice a pattern with the if statements, where you're doing

if response1 == "top" then
        selDrive = "top"
elseif response1 == "bottom" then
        selDrive = "bottom"
... and so on

You can replace this all with the following:

if response1 == "top" or response1 == "bottom" or ... then
        selDrive = response1
end

The issue is that this will make a really long if statement, and won't ensure that a disk is present. By using disk.hasData( response1 ) you're checking not that the side entered is actually a side, but that the side entered has a disk. This is better, and removes the need for the long if statement. The resultant code would be

if disk.hasData( response1 ) then
        selDrive = response1
end

Regarding locals, global variables (essentially ones defined without the word "local" before them) are considered bad practise for a multitude of reasons.
As such, the variables `selDrive`, `response1`, `file` and `dir` should all have `local` written before them *when they're defined* (not when they're used) (so when e.g. file=… for the first time).

Not that it's important, but it's conventional to put `then` after the condition of an if statement rather than on the next line, but this makes no practical difference.

Exerro, on 10 August 2017 - 08:11 PM said:

The Crazy Phoenix, on 10 August 2017 - 08:02 AM said:

What's wrong with setting selDrive = response1 (or replacing all instances of selDrive with response1)?

If you want to validate the user's input, try disk.hasData(response1). If it returns true, that means that the disk drive exists and contains a floppy disk.

Dave-ee Jones, on 10 August 2017 - 07:16 AM said:

-snip-

Notice how "top" is meant to be as a string. What your program is erroring out on is it thinks "Oh, he wants to make selDrive the same as top, let me just find it…wait, there is no top…whut?" and crashes. Without the quote marks it treats it as a variable, and since "top" was never initialised it thinks it's nil (uncalled), erroring. Use strings. Do the same for all the others as well (bottom, front etc.).

Use local variables (like I did in my code) as well.

To elaborate on these…
When you write `selDrive = top`, Lua's looking for a variable called `top`, and finding nothing, so this is equivalent to `selDrive = nil`, which clearly isn't what you want. To fix this, you can put parenthesis around `top`: `selDrive = "top".
Having done this, you'll notice a pattern with the if statements, where you're doing

if response1 == "top" then
		selDrive = "top"
elseif response1 == "bottom" then
		selDrive = "bottom"
... and so on

You can replace this all with the following:

if response1 == "top" or response1 == "bottom" or ... then
		selDrive = response1
end

The issue is that this will make a really long if statement, and won't ensure that a disk is present. By using disk.hasData( response1 ) you're checking not that the side entered is actually a side, but that the side entered has a disk. This is better, and removes the need for the long if statement. The resultant code would be

if disk.hasData( response1 ) then
		selDrive = response1
end

Regarding locals, global variables (essentially ones defined without the word "local" before them) are considered bad practise for a multitude of reasons.
As such, the variables `selDrive`, `response1`, `file` and `dir` should all have `local` written before them *when they're defined* (not when they're used) (so when e.g. file=… for the first time).

Not that it's important, but it's conventional to put `then` after the condition of an if statement rather than on the next line, but this makes no practical difference.

You literally just copied my post..

Dave-ee Jones, on 10 August 2017 - 11:33 PM said:

-snip-

You literally just copied my post..

It's just a different way of explaining the difference between variable names and string literals. Even though I already understood the subject, I personally found it clearer.