4
\$\begingroup\$

What I have

I am a bit advanced PHP developer, but now I'm going to college where we are learning Ruby. We got the first homework and I did it. But I wanna make great code - so I looked for standards and found pretty much nothing. I only realized, that variables aren't named with camelCase but_like_this. I edited some things, but still I don't think this is great. Can someone familiar with Ruby tell me, how to improve this code? I am interested in details, because I wanna learn the best practice at the beginning.

Specific questions

  1. Are functions without parameters bad? RubyMine highlights them and tells me "Parentheses around empty arguments list" but I am not sure, what does it mean.
  2. How would be this code divided into more files (what would go where)? // It was task of the homework to make only one file

Long story short - the task

There is someone who lives in 1526. He has chunks of wood, which he can burn to make heat. Then he has logs, which he can split into chunks. Day by day he has four options.

  1. Cut a tree (and get rand(2..4) logs )
  2. Split rand(2..5) logs into chunks and get two chunks for each log
  3. Go to sleep
  4. Die

When he does something the day ends and he burns one chunk. Every day I must write what day it is and how many logs and chunks we have.

My code

def print_status()
  puts "It's day #{$day}. You have #{$logs} logs and #{$chunks} of chunks."
end

def print_action()
  puts "\nWhat will you do? \n 1. Cut a tree \n 2. Split logs into chunks \n 3. Go to sleep \n 4. Die and end the simulation"
  puts "\nYour choice:"
end

def print_delimiter()
  puts "\n------------------------------------------------------------"
end

def user_interaction()
  print_delimiter()
  print_status()
  print_action()
end

def end_day()
  puts "\nDay has passed and you burned a single chunk."
  $day += 1
  $chunks -= 1
end

$day = 1
$logs = 0
$chunks = 3

puts 'You are a logger in 1526. Try to survive.'

while $chunks > 0 do

  user_interaction()
  choice = gets.to_i

  case choice
    when 1
      chopped_trees = rand(2..4)

      $logs += chopped_trees

      puts "\nYou cut #{chopped_trees} trees, you got #{chopped_trees} logs from them."

      end_day()
    when 2
      if $logs == 0
        puts "\nYou have no logs to split."
        end_day()
        next
      end

      logs_to_chop = rand(2..5)

      if logs_to_chop > $logs
        logs_to_chop = $logs
      end

      $logs -= logs_to_chop
      $chunks += logs_to_chop*2

      puts "\nYou split #{logs_to_chop} logs to #{logs_to_chop*2} chunks."

      end_day()
      next
    when 3
      puts "\nYou go to sleep."
      end_day()
      next
    when 4
      puts "\nYou died."
      exit
    else
      puts "\nInvalid input"
      next
  end
end

puts 'You run out of chunks. You froze to death.'
exit

Conclusion

I hope you will tell me those little details to make my code best practice. And by the way - the homework is not in English, so it's not important to correct my grammar mistakes.

\$\endgroup\$
1
\$\begingroup\$

Ruby is a departure from PHP. It has a different syntax. Parentheses are often optionally when calling functions. Say you have the following function:

def add(x, y)
    x + y
end

The following are equivalent:

puts(add(3, 5))
puts add(3, 5)
puts add 3, 5

Similarly, these parentheses can be omitted, but only when the arguments are empty. Thus, the following are equivalent:

def hello()
    puts "world"
end
def foo
    puts "bar"
end

hello
foo()

So, as for one of your code examples:

def user_interaction()
  print_delimiter()
  print_status()
  print_action()
end

Can become:

def user_interaction
  print_delimiter
  print_status
  print_action
end

def print_status()
  puts "It's day #{$day}. You have #{$logs} logs and #{$chunks} of chunks."
end

Minor English quibble: it should be #{$chunks} chunks.


  if logs_to_chop > $logs
    logs_to_chop = $logs
  end

You can use a neat feature of ruby's syntax: a single-statement if block shaped like this:

if c
    d
end

Can become this:

d if c

Accordingly, this line could become:

logs_to_chop = $logs if logs_to_chop > $logs

However, this change is to your discretion.


You should move your global variables ($logs, $day, $chunks) to the beginning of your code.


$chunks += logs_to_chop*2

You should always put a space on either side of binary operators like * and +. So, it should become this:

$chunks += logs_to_chop * 2

Your code ends with an exit statement; this is unnecessary, since the program terminates at the end of the program anyhow.


Other than these suggestions, I think your code is really readable. Great job! In larger programs, you may wish to add comments. I don't think they're as crucial here, since the task itself is simple enough.

\$\endgroup\$
1
\$\begingroup\$

next is not needed when it's the last statement in your loop. Neither is exit at the end of the program.

choice = gets.to_i

  case choice

could be case gets.to_i

  logs_to_chop = rand(2..5)

  if logs_to_chop > $logs
    logs_to_chop = $logs
  end

has a one-liner version logs_to_chop = [$logs, rand(2..5)].min

\$\endgroup\$
1
\$\begingroup\$

It's nice that you have defined functions. I would caution against overdoing it, though. The user_interaction is split up into three functions, each of which is only used within user_interaction. That just makes me jump around unnecessarily to understand the function.

To me, this constitutes a coherent, useful chunk of functionality, with validation that helps declutter the main program loop:

# Prints the game state (`$day`, `$logs`, and `$chunks`), then prompts
# the user to decide what to do next.
# 
# == Returns:
# A valid menu choice (1, 2, 3, or 4).
# 
def user_interaction
  loop do
    puts <<STATUS_MENU

------------------------------------------------------------
It's day #{$day}. You have #{$logs} logs and #{$chunks} chunks.

What will you do?
1. Cut a tree
2. Split logs into chunks
3. Go to sleep
4. Die and end the simulation

Your choice:
STATUS_MENU
    choice = gets.to_i
    return choice if (1..4) === choice
    puts "\nInvalid input"
  end
end

I've used a here document to make the menu more readable. Here's another way to write multi-line output:

def user_interaction
  loop do
    puts '',
         '------------------------------------------------------------',
         "It's day #{$day}. You have #{$logs} logs and #{$chunks} chunks.",
         '',
         'What will you do?',
         '1. Cut a tree',
         '2. Split logs into chunks',
         '3. Go to sleep',
         '4. Die and end the simulation',
         '',
         'Your choice:'
    choice = gets.to_i
    return choice if (1..4) === choice
  end
end

Since end_day() is common to most of the cases, I recommend factoring it out, like this:

while $chunks > 0 do
  case user_interaction()
  when 1    # Chop tree
    …

  when 2    # Split log
    if $logs == 0
      puts "\nYou have no logs to split."
    else
      logs_to_chop = [rand(2..5), $logs].min
      $logs -= logs_to_chop
      $chunks += 2 * logs_to_chop
      puts "\nYou split #{logs_to_chop} logs into #{2 * logs_to_chop} chunks."
    end

  when 3    # Sleep
    …

  when 4    # Suicide
    puts "\nYou died."
    exit

  else
    puts "\nInvalid input"
    redo
  end
  end_day()
end

Note the use of redo instead of next for invalid input. However, with user_interaction having taken care of input validation, we would no longer have to worry about it in the main loop.

Strictly speaking, the parentheses are optional when calling a function with no parameters. However, I often prefer to use the parentheses anyway, since it helps distinguish variables and function calls visually.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.