patternrubyModerate
Creating a 2D array of map tiles
Viewed 0 times
creatingtilesmaparray
Problem
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.
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
endSolution
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
No need to call
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
attr_accessor or the contents of the initialize method). You should fix that.attr_accessor :tileSprite, :attribute
# ...
def tileSprite
@tileSprite
end
def attribute
@attribute
endNo 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
endCreating 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
endCode Snippets
attr_accessor :tileSprite, :attribute
# ...
def tileSprite
@tileSprite
end
def attribute
@attribute
enddef array2D(width,height)
a = Array.new(width, MapTile.new(123,0))
a.map! { Array.new(height, MapTile.new(123,0)) }
return a
enddef array2D(width,height)
Array.new(width) do
Array.new(height) do
MapTile.new(123,0)
end
end
endContext
StackExchange Code Review Q#10550, answer score: 12
Revisions (0)
No revisions yet.