HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Improving this TileMap

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thistilemapimproving

Problem

I created a TileMap-class to load tile sets, create and display tile maps. It is far from perfect so I want your opinions on how to improve it!

How it works:

  • Create TileMap object with "new TileMap( int tilesize )".



  • Create a tile set with "createSet( String path )" and specify the tile types with "setTileSet( int row, int col, Integer type )". This tile get's saved in a HashMap to later access it by it's type-variable.



  • Create a tile map from a file with "createMap( String path )". I want to add in another method to create an empty tile map which you then can edit with "setTileMap( int row, int col, Integer type )" and output this map as a file to later load it (got it working but it's buggy).



I created a second class Map which you can use to output a map file. Please also take a look at that.
(Problem with that: I made a simple method in TileMap called write(path, Tile[][]) which created a Map object and then outputted it but it was buggy. I loaded a map and let the same map be written by this method but it came out differently. Rows and columns were exchange and the whole map were turned 90°. Appreciate your help on that one too).

The Code: I got rid of all JavaDoc since I did my best to explain it above. The code also isn't done so you can find some of my annotations about future ideas in there.

TileMap class:

```
// Settings
private int tilesize;

// TileSet
private BufferedImage sourceImage;
private HashMap tileList;

// TileMap
private Tile[][] tileMap;
private int mapRows;
private int mapCols;

public TileMap( int tilesize ) {
this.tilesize = tilesize;
}

public void createSet( String path ) throws IOException {
sourceImage = ImageIO.read( getClass().getResourceAsStream( path ) );
tileList = new HashMap();
}

public void setTileSet( int row, int col, Integer type ) {
BufferedImage subImgae = sourceImage.getSubimage( rowtilesize, coltilesize, tilesize, tilesize );
tileList.put( type, new Tile( subImgae, type ) );
}

public

Solution

Please choose better names

Don't "reuse" standard lib class names

You named one of your classes Mapbut there is already a class in the JVM having that name (the Map interface)

Don't confuse your readers

-
tileMap
your central variable is called tileMap but it is an array.

I understand, that the term map comes from your business case where the term map relates to a 2 dimensional rasterized representation of a geographical plane. But programmers understand by the term map a collection type that assignes values to keys.

So you should find a better name for tileMap e.g. simply tiles or gameField or alike...

-
setTileMap

You have a method called setTileMap. This is what programmers call a setter method. It is expected to put the value passed as parameter directly into a member variable of the class.

But in your case it behaves differently.

This method should better be named setTileAtPosition()

Context

StackExchange Code Review Q#151186, answer score: 3

Revisions (0)

No revisions yet.