triplej
triplej

Reputation: 299

When I make a second instance of a class in Ruby it changes the first instance of that class

I have a class that randomly creates a two dimensional array. I am testing this program in irb. When I create the first instance of a class by doing thing1 = CatanBoard.new everything works fine. When I create a second instance by doing thing2 = CatanBoard.new I run into problems. This causes thing1.board to be identical to thing2.board and it adds elements to the arrays in thing1.

Correct output of thing1.board after the initialization of thing1 looks like

[[2, 8, "wheat"], [4, 8, "forest"], [15, 6, "forest"], [12, 6, "stone"], [19, 12,       "sheep"], [9, 11, "forest"], [17, 11, "stone"], [6, 10, "wheat"], [14, 10, "stone"], [18, 9, "wheat"], [11, 9, "sheep"], [10, 5, "forest"], [16, 5, "brick"], [1, 4, "sheep"], [13, 4, "brick"], [5, 3, "sheep"], [8, 3, "wheat"], [7, 2, "brick"], [3, nil, "desert"]]

Then when I initialize thing2 by doing thing2 = CatanBoard.new I get this as the value of thing2.board:

[[2, 8, "wheat", 8, 8, "wheat"], [4, 8, "forest", 10, 8, "stone"], [15, 6, "forest", 17, 6, "forest"], [12, 6, "stone", 19, 6, "brick"], [19, 12, "sheep", 14, 12, "brick"], [9, 11, "forest", 11, 11, "sheep"], [17, 11, "stone", 18, 11, "stone"], [6, 10, "wheat", 16, 10, "wheat"], [14, 10, "stone", 12, 10, "brick"], [18, 9, "wheat", 13, 9, "forest"], [11, 9, "sheep", 5, 9, "wheat"], [10, 5, "forest", 2, 5, "sheep"], [16, 5, "brick", 1, 5, "sheep"], [1, 4, "sheep", 6, 4, "stone"], [13, 4, "brick", 9, 4, "wheat"], [5, 3, "sheep", 15, 3, "forest"], [8, 3, "wheat", 4, 3, "forest"], [7, 2, "brick", 3, 2, "sheep"], [3, nil, "desert", 7, nil, "desert"]]

Then I check the value of thing1.board and it is now the same as thing2.board:

[[2, 8, "wheat", 8, 8, "wheat"], [4, 8, "forest", 10, 8, "stone"], [15, 6, "forest", 17, 6, "forest"], [12, 6, "stone", 19, 6, "brick"], [19, 12, "sheep", 14, 12, "brick"], [9, 11, "forest", 11, 11, "sheep"], [17, 11, "stone", 18, 11, "stone"], [6, 10, "wheat", 16, 10, "wheat"], [14, 10, "stone", 12, 10, "brick"], [18, 9, "wheat", 13, 9, "forest"], [11, 9, "sheep", 5, 9, "wheat"], [10, 5, "forest", 2, 5, "sheep"], [16, 5, "brick", 1, 5, "sheep"], [1, 4, "sheep", 6, 4, "stone"], [13, 4, "brick", 9, 4, "wheat"], [5, 3, "sheep", 15, 3, "forest"], [8, 3, "wheat", 4, 3, "forest"], [7, 2, "brick", 3, 2, "sheep"], [3, nil, "desert", 7, nil, "desert"]]

This post had a similar problem, but since I'm using Array.new I don't think it's the same problem, since I am creating deep copies. What do you think the problem is with my code?

Here is my code:

# The class CatanBoard represents a Catan Board with no expansions
# A Catan board has 19 hexagons. Each hexagon has a roll and a resource on it. 
# Rolls that are 6's and 8's cannot be adjacent to other 6's and 8's
# The 'desert' square has no roll on it

RESOURCES = ['forest', 'forest', 'forest', 'forest', 'brick', 'brick', 'brick', 'wheat', 'wheat', 'wheat', 'wheat', 'sheep', 'sheep', 'sheep', 'sheep', 'stone', 'stone', 'stone'] # desert is left out because it isn't a resource and needs to be specially added

# note - there are 19 tiles, yet 18 rolls. This is because the desert tile does not get a roll. 
SPECIAL_ROLLS = [6, 6, 8, 8]
PLAIN_ROLLS = [2, 3, 3, 4, 4, 5, 5, 9, 9, 10, 10, 11, 11, 12]
TILES = (1..19).to_a
EMPTY_BOARD = Array.new(19) {Array.new(0) {[]}}
HARBORS = ['brick', 'generic', 'generic', 'generic', 'generic', 'sheep', 'stone', 'wheat', 'wood'] 

# this function returns the adjacent tiles on the catan board. 
# See the picture in "catan overview.odg" for details
def neighbors(loc)
  case loc
  when 1
    return [2, 4, 5]
  when 2
    return [1, 3, 5, 6]
  when 3
    return [2, 6, 7]
  when 4
    return [1, 5, 8, 9]
  when 5
    return [1, 2, 4, 6, 9, 10]
  when 6 
    return [2 , 3, 5, 7, 10, 11]
  when 7
    return [3, 6, 11, 12]
  when 8
    return [4, 9, 13]
  when 9
    return [4, 5, 8, 10, 13, 14]
  when 10
    return [5, 6, 9, 11, 14, 15]
  when 11
    return [6, 7, 10, 12, 15, 16]
  when 12
    return [7, 11, 16]
  when 13
    return [9, 14, 17]
  when 14
    return [9, 10, 13, 15, 17, 18]
  when 15
    return [10, 11, 14, 16, 18, 19]
  when 16
    return [11, 12, 15, 19]
  when 17
    return [13, 14, 18]
  when 18
    return [14, 15, 17, 19]
  when 19
    return [15, 16, 18]
  else
    return "error"
  end
end


class CatanBoard
  def initialize()

    # @board and @harbors are tha arrays that represent the games
    # The other variables are used to set up the board 
    @board = Array.new(EMPTY_BOARD) 
    @harbors = Array.new(HARBORS)  # @harbors[0] corresponds with A, while @harbors[8] corresponds with I in the harbor diagram in "catan overview.odg"
    @resources = Array.new(RESOURCES)
    @special_rolls = Array.new(SPECIAL_ROLLS)
    @plain_rolls = Array.new(PLAIN_ROLLS)
    @tiles = Array.new(TILES)

    # RANDOMIZE THE HARBORS #
    @harbors = @harbors.shuffle

    # PLACE THE SPECIAL ROLLS #
    temp_tiles = @tiles
    for i in (0..@special_rolls.length-1)
      loc = temp_tiles.delete_at(rand(temp_tiles.length)) # chooses a random tile as the location and saves it
            #temp_tiles.delete(loc) # I think this line isn't needed
      temp_tiles = temp_tiles - neighbors(loc)
    # puts the tile and the roll onto the board
      @board[i] << loc
      @board[i] << @special_rolls.pop 
      @tiles.delete(loc)
    end


    # THEN PLACE THE REST OF THE ROLLS #
    for i in ([email protected])
      loc = @tiles.delete_at(rand(@tiles.length))
      @board[i+4] << loc # +4 because loctions 0 through 3 are filled
      @board[i+4] << @plain_rolls.pop
    end


    # THEN PLACE THE RESOURCES #
    @board[@board.length-1] << 'desert' # matches the desert tile to the nil roll
    for r in ([email protected])
      # removes a random resource and pairs it with a location and a roll
      # resources must be removed randomly otherwise the 6's and 8's are all on stone and sheep
      @board[r] << @resources.delete_at(rand(@resources.length)) 
    end

  end

  def board()
    return @board
  end

  def harbors()
    return @harbors
  end



  # This determines if the board is set up according to the game's rules
  def is_legal?()

    #TEST SPECIAL ROLLS#
    # special rolls are in first four locations
    for loc in (0..3)
      for other_loc in (0..3)
        if neighbors(@board[loc][0]).include?(!@board[other_loc][0])
          return false
        end
      end
    end

    #TEST TOTAL ROLLS#
    all_rolls = Array.new(SPECIAL_ROLLS + PLAIN_ROLLS)

    # extracts the rolls from board
    temp_rolls = []
    # -2 because the desert square doesn't have a roll and it is last in the array
    for i in ([email protected])
      temp_rolls << @board[i][1]
    end

    temp_rolls = temp_rolls.sort
    all_rolls = all_rolls.sort

    if temp_rolls != all_rolls
      temp_resources
      return false
    end


    #TEST RESOURCE AMOUNT#

    all_resources = Array.new(RESOURCES)
    temp_resources = []
    # -2 because the desert square isn't a resource and it is last in the array
    for i in ([email protected]) 
      temp_resources << @board[i][2]
    end

    temp_resources = temp_resources.sort
    all_resources = all_resources.sort


    if temp_resources != all_resources
      return false
    end

    #TEST HARBORS#

    temp_harbors = Array.new(HARBORS)
    sorted_harbors = @harbors.sort

    if temp_harbors != sorted_harbors
      return false
    end

    return true
  end

end

Upvotes: 0

Views: 72

Answers (2)

Peter Alfvin
Peter Alfvin

Reputation: 29439

Array.new does not create a deep copy, so your boards are shared, since EMPTY_BOARD is two levels deep.

In addition, keeping EMPTY_BOARD outside of your CatanBoard class probably isn't a good idea anyway from a cohesion point of view. If you introduce a private method

def empty_board
  Array.new(19) {Array.new(0) {[]}}
end
private :empty_board

then you can just reference this method during initialization:

@board = empty_board

Upvotes: 2

Ismael
Ismael

Reputation: 16730

Instead Array.new(EMPTY_BOARD) do a deep copy. Here is an example how you can do it since there is no standard way.

def deep_copy(obj)
  Marshal.load(Marshal.dump(obj))
end

@board = deep_copy(EMPTY_BOARD) 

Upvotes: 0

Related Questions