Over the next few days, I'm planning to code review one of the very first programs I ever wrote, 'Twelve Labors'. It's a rough, basic stab at the fundamentals of programming, still open source on Github:
Here's the gist of this program, from the
This is a game I wrote as an assignment for Zed Shaw's book Learn Ruby The Hard Way. It is a command-line RPG in the style of early classics like 'Zork'.
Twelve-labors was inspired by the Ancient Greek hero Heracles (Hercules), who completed a series of superhuman feats, and in the process gained immortality.
Essentially, the user recreates the tasks that Hercules had to complete to gain his immortality. Choose the correct action and the character advances. Choose the wrong action, and the character dies.
I wrote this code as part of Learn Ruby The Hard Way by Zed Shaw. I credit this book with much of my early passion for programming. Zed taught me to type out all code by hand, to push through confusion, and to write tests.
Why review my own code now, almost four years later?
Well, I see a lot of newbie code in my role as part of the Hashrocket hiring team. And I find the same antipatterns in that code again and again. My goal is to use my own newbie code to think about the feedback I give and crystallize the words I use.
Today I'll be looking at the first file in the program,
README declares that to start the program, we type:
$ ruby start.rb
I'm already annoyed. This should be an executable.
start.rb. There's not a lot going on here, but I'd still change most of it.
# start.rb require_relative "level.rb" require_relative "lion.rb" require_relative "hydra.rb" require_relative "hind.rb" require_relative "boar.rb" require_relative "stables.rb" require_relative "birds.rb" require_relative "bull.rb" require_relative "mares.rb" require_relative "girdle.rb" require_relative "apples.rb" require_relative "cerebus.rb" require_relative "immortality.rb" a_game = Lion.new a_game.play
The first group of lines requires every file, each representing a level. But looking into each file, there are numerous ways to exit the program early, rendering all subsequent levels useless. It would be better to require only the next level at each stage, so that our computer's memory isn't loaded with potentially useless information.
I'd keep the
level.rb require, which is the parent class for every level. I'd also
use single quotes and ditch the
require_relative in favor of
./ for no
other reason than personal preference.
The file ends by assigning a variable
a_game, then calling
.play on that
object. My issue with this code:
a_gameis not a good name. Adding
a_accomplishes nothing and is not conventional Ruby. It should be
a_gamedoes not need to exist– we can call
Here is the final file, after these changes:
# start #!/usr/bin/env ruby require './level' require './lion' Lion.new.play
Next post, I will be looking at Level One,