Why is my ruby method always returning true?

Question

From codeschool's ruby-bits course, I am trying to understand how these classes work- I have a Game class and a collection class called Library that stores a collection of games.

class Game
  attr_accessor :name, :year, :system
  attr_reader :created_at

  def initialize(name, options={})
    self.name = name
    self.year = options[:year]
    self.system = options[:system]
    @created_at = Time.now
  end


  def ==(game)
    name == game.name && 
    system == game.system &&
    year == game.year
  end
end

Library class:

class Library
  attr_accessor :games

  def initialize(*games)
    self.games = games
  end

  def has_game?(*games)
    for game in self.games
      return true if game == game
    end
    false
  end
end

Now I create some games:

contra = Game.new('Contra', {
  year: 1994,
  system: 'nintendo'
})

mario = Game.new('Mario', {
  year: 1996,
  system: 'SNES'
})

sonic = Game.new('Sonic', {
  year: 1993,
  system: 'SEGA'
})

and instantiate a new collection:

myCollection = Library.new(mario, sonic)

When I try to find if a certain game is in myCollection using the has_game? method, I always get true

puts myCollection.has_game?(contra) #=> returns **true** even though this has never been inserted as part of the collection.

What am I doing wrong?

So is the `==(games)` instance method in the Game class that's the problem? How do I fix this? I need to check if the game is part of the collection or not — Amit Erandole, Dec 18 '12 at 16:12

score 2 · Answer 1 · edited Dec 18 '12 at 16:23

2

return true if game == game

I think this statement may cause problem.

It is always true.

You may want something like this:

def has_game?(wanted)
  for game in self.games
    return true if game == wanted
  end
  false
end

edited Dec 18 '12 at 16:23

Amit Erandole

11,336
20
64
98

answered Dec 18 '12 at 16:07

Salih Erikci

4,976
11
36
68

score 1 · Accepted Answer · edited May 23 '17 at 11:56

There are a couple things that are wrong here:

Instead of using self.XXXX to create instance variables you should use @XXXX, it accesses the values directly, using self actually performs another method call, refer here for more details: Instance variable: self vs @
As the others mentioned game == game will always return true, the answer that was already posted doesn't allow for passing more than a single game to has_game?

Here are my changes that work correctly:

class Game
  attr_accessor :name, :year, :system
  attr_reader :created_at

  def initialize(name, options={})
    @name       = name
    @year       = options[:year]
    @system     = options[:system]
    @created_at = Time.now
  end


  def ==(game)
    @name == game.name && 
    @system == game.system &&
    @year == game.year
  end
end

class Library
  attr_accessor :games

  def initialize(*games)
    @games = games
  end

  # only returns true if this Library
  # has ALL of the games passed to has_game? 
  def has_game?(*_games)
    _games.each do |game|
      return false if not @games.include?(game)
    end

    return true
  end
end

contra = Game.new('Contra', {
  year: 1994,
  system: 'nintendo'
})

mario = Game.new('Mario', {
  year: 1996,
  system: 'SNES'
})

sonic = Game.new('Sonic', {
  year: 1993,
  system: 'SEGA'
})

myCollection = Library.new(mario, sonic)
puts "Collection has Contra? #{myCollection.has_game?(contra)}"
puts "Collection has Sonic and Mario #{myCollection.has_game?(sonic, mario)}"

output:

Collection has Contra? false
Collection has Sonic and Mario true

your "has_game?()" method returns only whether the last game is included in @games... it would probably be easier with an array intersect, or you could iterate thus: def has_game?(*_games);_games.map{|game|@games.include?(game)}.all?;end — Pavling, Dec 18 '12 at 20:30
@Pavling You are right, I forgot to break out in the event of false. — Hunter McMillen, Dec 18 '12 at 20:31
@Pavling I ended up re-writing the method in a better way, thanks! — Hunter McMillen, Dec 18 '12 at 20:35

Why is my ruby method always returning true?

2 Answers2