# Creating a 2D array of map tiles

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

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

• 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
• @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
• @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
• @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
• @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

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) }


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

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

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){ [] }