Creating a 2D array of map tiles

Question

I am a C++ programmer new to Ruby and someone suggested that I post my code here since it doesn't quite match Ruby standards. Is there a more Ruby way of doing this?

I have created a 2D array full of MapTile class objects and then I draw the tiles to the screen by iterating through the 2D Array using a for loop. Here is some code.

class MapTile
attr_accessor :tileSprite, :attribute

    def initialize(sprite, attr)
    @tileSprite = sprite
    @attribute = attr
    end 

    def tileSprite
        @tileSprite
    end

    def attribute
        @attribute
    end

end

def array2D(width,height)
  a = Array.new(width, MapTile.new(123,0))
  a.map! { Array.new(height, MapTile.new(123,0)) }
  return a
end

@mapData = array2D(@mapSize,@mapSize)

for i in 0..@mapSize - 1
   for j in 0..@mapSize - 1
      mapData[i][j].tileSprite = tileNum
      @tile.draw(mapData[i][j].tileSprite)
   end 
end

"for" blocks are kind of code smoke in Ruby. Whenever you find yourself writing one, you should think again. Alternative can be using @mapSize.times { |i| some_operation } — Can Hanhan, Apr 1 '12 at 21:06

200_success · Accepted Answer · 2014-10-16 19:03:28Z

up vote 12 down vote accepted

First of all your indentation is a bit wacky. Sometimes you indent by 4 spaces, sometimes by 2 (which is the default in the ruby community) and sometimes not at all (for example you didn't indent the call to attr_accessor or the contents of the initialize method). You should fix that.

attr_accessor :tileSprite, :attribute
# ...
def tileSprite
    @tileSprite
end

def attribute
    @attribute
end

No need to call attr_accessor and define the getter methods yourself. Do one or the other.

def array2D(width,height)
  a = Array.new(width, MapTile.new(123,0))
  a.map! { Array.new(height, MapTile.new(123,0)) }
  return a
end

Creating an array containing a default value and then mapping over that array to replace the default value makes very little sense. Why assign a default value at all if you throw it away right after?

Also it's a very bad idea to make the inner arrays all contain a reference to the same map tile. This just screams for you to introduce hard to find bugs. As a guide line you should never use the 2-argument version of Array.new with a mutable value as its argument. Use the block-version instead.

Also it's customary in ruby to not use explicit return statements in the last line of a method.

Here's how I'd write the array2d method:

def array2D(width,height)
  Array.new(width) do
    Array.new(height) do
      MapTile.new(123,0)
    end
  end
end

edited Oct 16 '14 at 19:03

200_success♦

97k11114333

answered Apr 1 '12 at 20:15

sepp2k

8,19222544

do you really have to build multidimensional arrays with the multi step way you did? I used the 2 argument array style to create them and have problems; stackoverflow.com/questions/27064163/… Is there not a cleaner way? – Vass Nov 25 '14 at 17:07

1

@Vass Yes, you need to use the block version of Array.new. If you use the two-argument version, you're creating an array containing n references to the same subarray rather than n independent subarrays. – sepp2k Nov 25 '14 at 18:48

1

@Vass Sure, that's exactly what the OP did with map! (and I have to say: I much prefer map! over each_index to change elements in an array), but why would you? You're still using a block - you just replaced one method call with two. – sepp2k Nov 26 '14 at 13:28

1

@Vass map can be nested just like each_index, so can Array.new, which is still the best solution. – sepp2k Nov 26 '14 at 13:50

1

@Vass Yes, it works, but why would you do that? Why call an additional method instead of passing the block directly to Array.new? That serves absolutely zero purpose. – sepp2k Nov 26 '14 at 14:09

| show 3 more comments

200_success · Answer 2 · 2014-10-16 19:04:59Z

I added indexes and move tileNum assignment to array2D function. You should normally use better names like "create_map_data"

I replaced the for-loops as nothing seems to be done based on indexes there. You should also avoid using for loops in Ruby. Using ranges mostly provide more readable code.

class MapTile
  attr_accessor :tileSprite, :attribute
  def initialize(sprite, attr)
    @tileSprite = sprite
    @attribute = attr
  end
end

def create_map_data(width,height)
  Array.new(width) do |i|
    Array.new(height) do |j| 
      MapTile.new(tileNum,0) # I guess tileNum somehow depends on indexes
    end
  end
end

@mapData = create_map_data(@mapSize,@mapSize)
@mapData.flatten.each { |cell| @tile.draw(cell.tileSprite) }

knut · Answer 3 · 2012-04-01 20:50:20Z

sepp2k gave you already an answer for a 2D-Array.

I have another solution with a new class Board.

You may define a board and have access to all fields, selected rows, columns or fields.

class MapTile
    attr_accessor :tileSprite, :attribute

    def initialize(sprite, attr)
      @tileSprite = sprite
      @attribute = attr
    end 
    def to_s
      "<MapTile #{object_id}>"
    end
end

class Board

  attr_reader :width,:height
  def initialize(width,height)
    @width = width
    @height = height
    @fields = Hash.new
  end
  #Make an initial filling
  def fill(default=nil)
    1.upto(@width){|i|
      1.upto(@height){|j|
        @fields[[i,j]] = yield if block_given?
      }
    }
  end  
  #Loop on all fields
  def each()
    1.upto(@width){|i|
      1.upto(@height){|j|
        yield @fields[[i,j]]
      }
    }
  end
  #Loop on all fields
  def each_with_pos()
    1.upto(@width){|i|
      1.upto(@height){|j|
        yield i,j,@fields[[i,j]]
      }
    }
  end
  #Each fields in a row
  def row(row)
    res = []
    1.upto(@height){|j|
      res << @fields[[row,j]]
      yield @fields[[row,j]] if block_given?
    }
    res
  end
  #Each fields in a column
  def column(col)
    res = []
    1.upto(@width){|i|
      res << @fields[[i,col]] 
      yield @fields[[i,col]]  if block_given?
    }
    res
  end
  #Access one field
  def [](*key)
    @fields[key]
  end
end
#Define the board
board = Board.new(5,5)

#Make an initial filling
board.fill{   MapTile.new(123,0) }

#Loop on all fields
board.each{|field|
  #~ p field
}
board.each_with_pos{|i,j,field|
  puts "%2i-%2i: #{field}" %  [i,j]
}
#Row 1
p board.row(1)
#Column 1
p board.column(1)
#Get on field
p board[1,2]

Take it as an example how you could solve it. If you have problems to understand the code, please ask.

isn't there a better more concise way to make multidimensional arrays? — Vass, Nov 25 '14 at 17:08

Sandip Ransing · Answer 4 · 2012-04-03 09:41:09Z

up vote -1 down vote

you don't need to write code for 2D array in ruby language. Just below declaration represent 2D array in ruby.

[[1,2], [3,4]]

OR Array of array

Array.new(3){ [] }

answered Apr 3 '12 at 9:41

Sandip Ransing

1113

add a comment |

asked	4 years ago
viewed	1738 times
active	1 year ago

current community

your communities

more stack exchange communities

Creating a 2D array of map tiles

4 Answers 4

Your Answer

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

Hot Network Questions

current community

your communities

more stack exchange communities

Creating a 2D array of map tiles

4 Answers 4

Your Answer

Sign up or log in

Post as a guest

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

Related

Hot Network Questions