4
\$\begingroup\$

I cannot understand how I can make two classes work together by passing variables between them.

During the last 5 days, I learned the basics of inheritance, singleton methods and eval methods. I even read about template methods, but since I have no programming background, I'm in no position to learn from other languages.

The only aim here is to learn how to make use of classes. The whole game is an exercise.

The game has 5 classes: Game, Creature, Weapon, Armor, Scene.

Creature class is used to produce the hero and several monsters (currently only a dragon is created).

Weapon and Armor classes (with only name and power attributes) are there to produce equipment and while they may be joined as a single Equipment class with name, defense and attack attributes, currently this is not my concern.

Scene class is there to produce several places/rooms, which is the main point of the exercise. The exercise clearly states "Use one class per room and give the classes names that fit their purpose." Therefore I made a class containing name, history and armor/weapon and monster attributes. The reason for name and history are clearly to be able to give an introduction when the player enters the scene.

The armor/weapon and monster attributes are required to make the player to encounter different armors, weapons to equip and different monsters to fight in different scenes. I tried to pass them as variables (arrays) to the scene class during initialize but couldn't succeed. The roots of this failure probably is the reason why I can't make the two classes work together.

I have to admit that I had a hard time to understand how to pass variables to/between methods in ruby, and this curse is following me in the classes, too.

So, as a last thing, you can see the horrible code in the choices method in Scene class. I cannot exactly explain what I was thinking while coding this. But the only good thing I have done is adding self to the @city = Scene.new step of the Game.initialize(), so that I can pass the game object to the city object. After that I got lost and need your help.

#game.rb
class Game
 attr_accessor :armor_list, :weapon_list, :equipment
 def initialize()
 #Game creates the city and mountain as two separate scenes
 @city = Scene.new("City", "The city.", @cm, @ls, "", self)
 @mountain = Scene.new("Mountain", "The mountain.", @em, @wh, @d, self)
 #Game creates armor and weapon array-name-holders, I couldn't find another way to make the equipment-selection work
 @a = Armor.new("armor_list", 0)
 @w = Weapon.new("weapon_list", 0)
 #Game creates weapons
 @ss = Weapon.new("Short sword", 5)
 @ls = Weapon.new("Long sword", 8)
 @ts = Weapon.new("Two-Handed sword", 12)
 @wh = Weapon.new("Warhammer", 10)
 #Game creates armors
 @la = Armor.new("Leather Armor", 5)
 @cm = Armor.new("Chain Mail", 10)
 @pm = Armor.new("Plate Mail", 15)
 @em = Armor.new("Elven Chain Mail", 50)
 #Game creates a hero and a dragon
 @h = Creature.new(name = "You", level = 5)
 @d = Creature.new(name = "Dragon", level = 12)
 #The default equipment list to provide an arsenal. I wish to separate them and put 1 or 2 into each scene
 @armor_list = [@a, @la, @cm, @pm, @em]
 @weapon_list = [@w, @ss, @ls, @ts, @wh]
 @equipment = []
 end
 def play()
 intro()
 end
 # If I can find a way to pass arguments between scenes and game, I'm going to put a variable to change the starting point. Otherwise I know that both intro() and play() is useless.
 def intro()
 @city.intro()
 end
end
class Creature
 attr_accessor :name, :life, :armor, :weapon, :regen
 def initialize(name, level)
 #Attributes accoridng to level and dice
 @name = name
 @level = level
 @equipment = {:armor => nil, :weapon => nil}
 @armor = 0
 @weapon = 0
 #3.times rand(7) or 3*rand(7) doesn't create the effect, I tried rand(16)+3 but didn't like it.
 @strength = rand(7) + rand(7) + rand(7)
 @condition = rand(7) + rand(7) + rand(7)
 @life = @level * (rand(8) + 1)
 @power = @strength * (rand(4) + 1)
 @regen = @condition
 end
 def select_equipment(equipment)
 introduce_self()
 selection(equipment)
 choice = gets.chomp.to_i
 #@a and @w help to get the name of the array, armor or weapon. 
 if equipment[0].name == "armor_list"
 wear_armor(equipment[choice])
 elsif equipment[0].name == "weapon_list"
 wear_weapon(equipment[choice])
 else
 raise ArgumentError, "Should be armor_list or weapon_list"
 end
 equipment.delete_at(choice)
 end
 def introduce_self()
 if @equipment[:armor] == nil
 puts "You wear no armor!"
 else
 puts "You wear #{@equipment[:armor]}."
 end
 if @equipment[:weapon] == nil
 puts "You carry no weapon!"
 else
 puts "You carry #{@equipment[:weapon]}."
 end 
 end 
 def selection(equipment)
 puts "You wanna some equipment?"
 for i in (1..equipment.length-1) do
 puts "#{i}. #{equipment[i].name}"
 i += 1
 end
 end 
 def wear_armor(armor) 
 @armor = armor.power
 @equipment[:armor] = armor.name
 end
 def wear_weapon(weapon)
 @weapon = weapon.power
 @equipment[:weapon] = weapon.name
 end
 def battle(opp1, opp2)
 #a basic round system depending on even and uneven numbers
 i = 1
 while opp1.life > 0 && opp2.life > 0
 if i % 2 == 0
 attack(opp1, opp2)
 elsif i % 2 == 1
 attack(opp2, opp1)
 else
 #just learning to raise errors, not into rescue, yet
 raise ArgumentError, "The battle is over!"
 end
 i += 1
 round_result(opp1, opp2)
 end
 end
 def round_result(opp1, opp2)
 puts "Hit points:"
 puts "#{opp1.name}: #{opp1.life}"
 puts "#{opp2.name}: #{opp2.life}"
 end
 def attack(attacker, defender)
 #this code below is just to prevent stuff like "hit with -27 points of damage"
 possible_attack = @power + @weapon - defender.armor
 if possible_attack > 0
 attack = possible_attack
 else
 attack = 0
 end
 defender.life -= attack
 puts "#{attacker.name} hit #{defender.name} with #{attack} points of damage!"
 if defender.life <= 0
 puts "...and killed!"
 defender.life = "Dead as cold stone!"
 round_result(attacker, defender)
 #game exits if one of the creatures die
 Process.exit(0)
 else
 defender.life += defender.regen
 puts "#{defender.name} regenerates #{defender.regen} points of life!"
 end
 end
end
#separate classes for weapons and armors, probably unnecessary but still learning
class Weapon
 attr_reader :name, :power
 def initialize(name, power)
 @name = name
 @power = power
 end
end
class Armor
 attr_reader :name, :power
 def initialize(name, power)
 @name = name
 @power = power
 end
end
# I want each scene have its own weapon or armor (scattered on the ground, maybe, according to the story..) but cannot achieve that with armors, weapon variables. The same thing applies to monsters. I would like to have for example rats and thieves in the city, but bats and a dragon in the mountain. However couldn't achieve this exactly. So far I'm only successful in passing the game object to the scene object as the last variable and I think that's a good start.
class Scene
 attr_reader :name, :history, :armors, :weapons, :monsters
 def initialize(name, history, armor_list, weapon_list, monsters, game)
 @name = name
 @history = history
 @armor_list ||= []
 @weapon_list ||= []
 @monsters ||= []
 @game = game
 end
 def intro()
 puts "You are in the " + @name + "."
 puts @history
 choices()
 end
 def choices()
 puts <<-CHOICES
 What would you like to do here?
 1. Look for armor
 2. Look for weapons
 3. Look for monsters to fight
 4. Go to another place!
 CHOICES
 choice = gets.chomp
 #this is where things go really bad! instance_variable_get saves the battle but as for the equipment selection,
 # as soon as I make a choice, it throws this error:
 # "game.rb:193:in 'choices': #<Armor:0x429720 @name="....> is not a symbol (TypeError)" 
 # and I don't think that further addition of : or @ is needed here.
 #The solution should be much simpler but couldn't find it on the web.
 if choice == "1" @game.send(@game.instance_variable_get(:@h).select_equipment(@game.instance_variable_get(:@armor_list)))
 elsif choice == "2" @game.send(@game.instance_variable_get(:@h).select_equipment(@game.instance_variable_get(:@weapon_list)))
 elsif choice == "3" @game.send(@game.instance_variable_get(:@h).battle(@game.instance_variable_get(:@h), @game.instance_variable_get(:@d)))
 elsif choice == "4"
 puts "bad choice, since I am not ready yet!"
 else
 puts "Can't you read?"
 end
 end
 #this is just to show the player a list of equipment found in the scene.
 def equipment_list()
 @armor_list[1..@armor_list.length-1].each {|a| @equipment.push(a.name) }
 @weapon_list[1..@weapon_list.length-1].each {|w| @equipment.push(w.name) }
 puts "You see some #{@room_equipment.join(", ")} lying on the ground."
 end
end
game = Game.new()
game.play()
RubberDuck
31.1k6 gold badges73 silver badges176 bronze badges
asked Mar 25, 2012 at 0:25
\$\endgroup\$

3 Answers 3

3
\$\begingroup\$

The problem is more about the organization of the game. Normally you would not want to have two classes tightly coupled to each other.

Also location of some methods are not correct. For example battle method in Creature should not be there. Or you should not need two parameters for that method.

You are suffering from the responsibilities of objects at the moment. I would suggest to rethink this.

For example you can decide that "Battle" is not something creature is responsible but game.

I came up with this code which also requires much refactoring. I tried not to change your code too much.

#game.rb
class World
 attr_accessor :armor_list, :weapon_list, :monsters, :scenes
 def initialize()
 @weapon_list = {
 short: {name: "Short sword", power: 5 },
 long: {name: "Long sword", power: 8 },
 two_handed: {name: "Two-Handed sword", power: 12 },
 warhammer: {name: "Warhammer sword", power: 10 } 
 }
 @armor_list = {
 leather: {name: "Leather Armor", power: 5 },
 chain: {name: "Chain Mail", power: 10 },
 plate: {name: "Plate Mail", power: 15 },
 elven: {name: "Elven Chain Mail", power: 50 } 
 }
 @monsters = { 
 rat: {name: "Rat", level: 1 },
 thief: {name: "Thief", level: 50 },
 dragon: {name: "Dragon", level: 12 }
 }
 @scenes = [
 { name: "City", history: "The city.", weapons: [@weapon_list[:long]], armors: [@armor_list[:chain]], monsters: [@monsters[:thief], @monsters[:rat]] },
 { name: "Mountain", history: "The mountain.", weapons: [@weapon_list[:warhammer]], armors: [@armor_list[:chain]], monsters: [@monsters[:dragon]] }
 ]
 end
end 
class Game
 def initialize
 @player = Creature.new name: "You", level: 5
 @world = World.new
 end 
 def play
 @current_scene = Scene.new(@world.scenes.first)
 @current_scene.intro
 while @player.is_alive?
 self.show_choices
 end
 end
 def show_choices
 puts <<-CHOICES
 What would you like to do here?
 1. Look for armor
 2. Look for weapons
 3. Look for monsters to fight
 4. Go to another place!
 CHOICES
 choice = gets.chomp
 case choice
 when "1"
 self.look_for_armor
 when "2"
 self.look_for_weapons
 when "3"
 self.look_for_monsters
 when "4"
 puts "bad choice, since I am not ready yet!" 
 else
 puts "Can't you read?"
 exit
 end
 end
 def look_for_monsters
 monster = @current_scene.monsters.first
 battle [@player, monster]
 end 
 def look_for_armor
 armor = select_equipment @current_scene.armors
 return if armor.nil?
 @player.wear_armor(@current_scene.armor_picked(armor)) 
 end 
 def look_for_weapons
 weapon = select_equipment @current_scene.weapons
 return if weapon.nil?
 @player.wear_weapon(@current_scene.weapon_picked(weapon))
 end
 def select_equipment(equipment)
 @player.introduce_self
 puts "You wanna some equipment?"
 equipment.each_with_index do |item, i|
 puts "#{i+1}. #{item.name}"
 end
 gets.chomp.to_i - 1
 end 
 def battle(opponents) 
 attack_turn = 0
 while opponents.all?(&:is_alive?)
 attacker = opponents[attack_turn]
 defender = opponents[(attack_turn + 1) % 2]
 attack = attacker.attack(defender)
 defender.defend(attack)
 puts "#{attacker.name} hit #{defender.name} with #{attack} points of damage!"
 puts "#{defender.name} regenerates #{defender.regen} points of life!" if defender.is_alive?
 puts "Hit points:"
 opponents.each { |o| puts "#{o.name}: #{o.life}" }
 attack_turn = (attack_turn + 1) % 2
 end
 winner = opponents.first(&:is_alive?)
 puts winner.name 
 end
end
class Scene 
 attr_reader :monsters, :armors, :weapons
 def initialize(setup)
 @name = setup[:name]
 @history = setup[:history]
 @armors = setup[:armors].map { |x| Armor.new x }
 @weapons = setup[:weapons].map { |x| Weapon.new x }
 @monsters = setup[:monsters].map { |x| Creature.new x }
 end
 def intro()
 puts "You are in the " + @name + "."
 puts @history
 equipment_list
 end
 def armor_picked(index)
 @armors.delete_at(index)
 end
 def weapon_picked(index)
 @weapons.delete_at(index)
 end
 def equipment_list()
 (@armors + @weapons).each {|a| puts "You see some #{a.name} lying on the ground." } 
 end
end
class Creature
 attr_reader :name, :life, :weapon, :armor
 def initialize(config)
 @name = config[:name]
 @level = config[:level]
 @equipment = {:armor => nil, :weapon => nil}
 @armor = 0
 @weapon = 0
 #3.times rand(7) or 3*rand(7) doesn't create the effect, I tried rand(16)+3 but didn't like it.
 @strength = rand(7) + rand(7) + rand(7)
 @condition = rand(7) + rand(7) + rand(7)
 @life = @level * (rand(8) + 1)
 @power = @strength * (rand(4) + 1)
 @regen = @condition
 end
 def introduce_self()
 puts "You wear " + (@equipment[:armor] || "no armor!")
 puts "You carry " + (@equipment[:weapon] || "no no weapon!")
 end 
 def wear_armor(armor) 
 @armor = armor.power
 @equipment[:armor] = armor.name
 end
 def wear_weapon(weapon)
 @weapon = weapon.power
 @equipment[:weapon] = weapon.name
 end
 def attack(defender) 
 [@power + @weapon - defender.armor, 0].max
 end
 def defend(attack)
 @life -= attack
 end
 def regen
 @life += @regen
 @regen
 end
 def is_alive?
 @life > 0
 end 
end
class Weapon
 attr_reader :name, :power
 def initialize(setup)
 @name = setup[:name]
 @power = setup[:power] end
end
class Armor
 attr_reader :name, :power
 def initialize(setup)
 @name = setup[:name]
 @power = setup[:power]
 end
end
game = Game.new()
game.play()
answered Mar 25, 2012 at 11:51
\$\endgroup\$
5
  • \$\begingroup\$ Good refactoring! Just one thing, you may want to change some of the methods that take on array to taking two arguments or a * (splat) argument. \$\endgroup\$ Commented Mar 25, 2012 at 13:44
  • \$\begingroup\$ I am examining your code with great pleasure. So I have 2 questions: 1) I see 99.9% of code in the classes and just 'game = Game.new().play()' to start the game. It's just like clicking start button on a web page. Is this the proper/preferred way to use classes? I mean, weapons, armors and monsters could be separate hashes outside classes, just before the 'game = ' part. 2) I'm a newbie and working on Zed Shaw's tutorial, exercise 43. Could you mind to tell me if the complexity of this game is appropriate to the level of the exercise? \$\endgroup\$ Commented Mar 25, 2012 at 14:17
  • \$\begingroup\$ @Linux_iOS.rb.cpp.c.lisp.m.sh you are right, habits from Javascript :) \$\endgroup\$ Commented Mar 25, 2012 at 14:38
  • \$\begingroup\$ @barerd I did not wanted to change your logic but only the "ruby" part. I could do the same without any classes. The usage of classes in my example is not really something you would like to have. The code outside is just "entry-point". \$\endgroup\$ Commented Mar 25, 2012 at 14:47
  • \$\begingroup\$ @barerd There are many things that you can do. For example instead of using hashes to setup the world; you can create separate classes for each equipment to give them different behaviours. (i.e: you can add a blade that can attack twice or a mice that will lower to regen; or a special armor that will hurt the attacker on random chance) You would put these classes in separate folders (./armors, ./weapons, ./scenes) and require and create in the game (runner class). I guess you are building something bigger than the author asked for. It is always hard to manage the interactions in the games. \$\endgroup\$ Commented Mar 25, 2012 at 14:51
2
\$\begingroup\$

Don't use instance_variable_get unless you really need it. And here you don't need it.

When you need the content of an attribute outside the class, then define a getter:

class Game
 attr_accessor :armor_list, :weapon_list, :equipment
 #additional getter
 attr_reader :h, :d
 #....continue

and then in approx line 200:

#this is where things go really bad! instance_variable_get saves the battle but as for the equipment selection,
# as soon as I make a choice, it throws this error:
# "game.rb:193:in 'choices': #<Armor:0x429720 @name="....> is not a symbol (TypeError)" 
# and I don't think that further addition of : or @ is needed here.
#The solution should be much simpler but couldn't find it on the web.
if choice == "1" 
 @game.h.select_equipment(@game.armor_list)
elsif choice == "2" 
 @game.h.select_equipment(@game.weapon_list)
elsif choice == "3" 
 @game.h.battle(@game.h, @game.d)
elsif choice == "4"
 puts "bad choice, since I am not ready yet!"
else
 puts "Can't you read?"
end

I wouldn't use h and d. Use hero and dragon (and you have to replace @h and @d in your program).

 #The hero (me)
 attr_reader :hero
 #The dragon
 attr_reader :dragon

If you have a problem, could you post a play sequence?


in approx line 90 you have:

def selection(equipment)
 puts "You wanna some equipment?"
 for i in (1..equipment.length-1) do
 puts "#{i}. #{equipment[i].name}"
 i += 1
 end
end 

Normally you should use:

def selection(equipment)
 puts "You wanna some equipment?"
 equipment[1..-1].each_with_index do |equip, i|
 puts "#{i}. #{equip.name}"
 end
end 

The [1..-1] is needed to skip the first element ("armor_list"). It is not a good idea to mix description and content inside one array. (Especially if your @equipment is a hash) I haven't analyzed your code in depth, but I think you should replace

def select_equipment(equipment)

with

def select_equipment(equipmentname, equipment)
answered Mar 25, 2012 at 9:15
\$\endgroup\$
1
  • \$\begingroup\$ Thank you for your reply: "When you need the content of an attribute outside the class" I wish I could see this when I read about getter and setter methods before. \$\endgroup\$ Commented Mar 25, 2012 at 20:26
1
\$\begingroup\$

Also, in addition to the other two answers, you should never use instance variables to store static content (such as the stats of a short sword). Not only does it imply that the static data (Weapon.new("Short Sword", 5)) will change between objects (I doubt that the starting stats of a weapon will change between objects). In addition to this, reinitializing the Weapon object is a wast of reassures to do every time.

Instead of instance varibles named after each weapon, here is one soulution:

@@weapon={ss: Weapon.new("Short Sword", 5), ls: Weapon.new("Longsword", 10), etc...}

Also, on lines 7 and 6, you are passing nil for every parameter starting with @. No instance variables have been declared yet, and Ruby gives nil as the value of initialized instance, class, and global variables. You must put that initialization after the instance variables are declared if you want any value other than nil (and if you want nil, just use the litteral).


Also, there is a slight matter of Ruby form here: When a method takes no arguments, omit the parenthesis unless it is an issue of ambiguity between names.


In the selection method, you should put the gets.chomp in the method and return it. A method that prints the prompt but doesn't read the data is not worth having.


In that battle method, don't throw an ArgumentError. The battle ending is no reason to think that the arguments are wrong. Instead, make your own error class:

class BattleOverException < StandardError; end

And throw it:

raise BattleOverException, "The battle is over!!"

Although this works, exceptions should be used for the comunication of an error at the file level (integer passed, expecting float, file not there, wrong number of arguments, etc.), not user problems (entered number expecting string, etc.). If you are using exceptions to exit the method, just use return. If you are trying to terminate the program, use puts for a message and use exit. And if you want to signal some condition to the calling code that isn't an error, take a look at [catch][1] and [throw][2].


round_result should not exist. There is no reason to factor out some simple printing to a method called once. The code is not complex enough to be its own method or enough of its own concept.


As for your problem of class communication, you can boil it down to this: Give the Game class the methods that the Scene class must use to interact with it. Continue passing self to the Scene constructor. Also, in the future, you may want to consider one manager class that manages all of the data, or a more decoupled class design. I recommend the second.

answered Mar 25, 2012 at 14:02
\$\endgroup\$
7
  • \$\begingroup\$ Thank you for these nice points. As I mentioned to @CanHanhan, I am a newbie and a physician. So I'm not going to write professional games. My interest in ruby stems from my efforts to organize patient/research data into mysql db, and rails made it very easy to create/use them. I just want to improve my grasp on the language, so that I can get rid of stealing from other people's code. So is there any document about decoupling classes you can suggest to me, which is simpler than this one? \$\endgroup\$ Commented Mar 25, 2012 at 14:37
  • \$\begingroup\$ I don't have one at my disposal at this moment (I should write one on my blog), but all I mean is that you should make your classes individual units: One class should not be controlling the object that created it, but objects should form a hierarchy. \$\endgroup\$ Commented Mar 25, 2012 at 14:45
  • \$\begingroup\$ Okay, I'll be watching your blog. I'm using gedit by the way and after I read your blog about it, I am going to add a ruby console to it. \$\endgroup\$ Commented Mar 25, 2012 at 16:14
  • \$\begingroup\$ I read Programming Ruby. And saw there: "..the only way to change an object's state in Ruby is by calling one of its methods. Control access to the methods and you've controlled access to the object. A good rule of thumb is never to expose methods that could leave an object in an invalid state." This made me think that in order to eliminate coupling, all methods capable to modify an object's attributes, must only be put into that object's class, and not into any other class. Is that what you mean? \$\endgroup\$ Commented Mar 25, 2012 at 20:22
  • \$\begingroup\$ True. But the main thing is two make it so that somewhat unrelated classes (maybe you want to use class Scene in another game) can be moved and used without the other. It is more about making things more general. (Post is almost done). \$\endgroup\$ Commented Mar 26, 2012 at 0:27

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.