An Update on my TileMap Class

Question

I'm making a old-school Final Fantasy type game and I made a tile map class a while back and posted it on here (Is this a good way to implement a tile map from a text file?) But, when I got to collision, it was a nightmare. So I've updated it to make it more collision friendly and I was wondering what the community's thoughts were on this code.

Tile.java

import java.awt.image.BufferedImage;
public class Tile 
{
 public int texKey; //number to represent texture
 public BufferedImage texture;
 public boolean collidable;
 public Tile(int texKey, BufferedImage texture, boolean collidable)
 {
 this.texKey = texKey;
 this.texture = texture;
 this.collidable = collidable;
 }
}

TileMaker.java

import com.stardust.rpgtest2.gfx.Assets;
public class TileMaker 
{
 public Tile[] tileArray= new Tile[28];
 public TileMaker()
 {
 }
 public void setTile()
 {
 for(int i = 0; i <= tileArray.length; i++)
 { 
 switch(i)
 {
 case 0:
 {
 tileArray[i] = new Tile(i, Assets.grass, false);
 break;
 }
 case 1:
 {
 tileArray[i] = new Tile(i, Assets.dirt, false);
 break;
 }
 case 2:
 {
 tileArray[i] = new Tile(i, Assets.water, false);
 break;
 }
 //this goes up to 27 at the moment
 default:
 {
 break;
 }
 }
 }
 }
 public Tile[] getTileArray()
 {
 return tileArray;
 }
}

TileMap.java

import com.stardust.rpgtest2.Game;
import java.awt.Graphics;
import java.awt.image.BufferedImage;
import java.io.BufferedReader;
import java.io.FileReader;
import java.io.IOException;
public class TileMap 
{
 int posX, posY, x, y, sx, sy, mapSize;
 public String filename;
 public int[][] tilemap;
 public TileMap(int posX, int posY, int mapSize, String filename)
 {
 this.mapSize = mapSize;
 tilemap = new int[mapSize][mapSize];
 this.posX = posX;
 this.posY = posY;
 this.sx = mapSize;
 this.sy = mapSize;
 this.filename = filename;
 }
 public void render(Graphics g)
 {
 for(int y = 0; y < tilemap.length; y++)
 { 
 for(int x = 0; x < tilemap.length; x++)
 {
 int textureType = tilemap[x][y];
 BufferedImage texture = Game.tileMaker.getTileArray()[textureType].texture; 
 g.drawImage(texture, posX, posY, null);
 posY += 32;
 }
 posX += 32;
 posY = Game.mapY;
 }
 posX = Game.mapX;
 posY = Game.mapY;
 }
 public void fileParser() throws IOException
 {
 BufferedReader in = new BufferedReader(new FileReader(filename));
 String line;
 while((line = in.readLine()) != null)
 {
 String[] values = line.split(",");
 for(String str : values)
 {
 int str_int = Integer.parseInt(str);
 tilemap[x][y] = str_int;
 ++y;
 }
 ++x;
 y = 0;
 }
 x = 0;
 in.close();
 }
}

I call the TileMaker.setTile() and TileMap.fileParser() methods both in the initialization method in the main game class, and everything behaves as needed so far.

• Twiggy: Updated Code codereview.stackexchange.com/questions/84868/…

Answer 1

• The for-case paradigm is an anti-pattern, you shouldn't use it. One alternative would be to initialize the array in a static block (without for-switch), another would be to use an enum. I would prefer the second approach. Both approaches would save you the call of TileMaker.setTile().
• Your indent style is not customary in Java, and it's always best to go with what most programmers of a language are used to (opening curly bracket on the same line).
• Your variable names are not very clear. What is the difference between posX and x? And what is sx?
• Get rid of unused fields. If you don't need mapSize, don't save it.
• Move magic numbers out of your code into static fields.
• I would create a separate class for the parsing of the file.

• Twiggy: Question edited to explain posX, x, sx, and mapSize.
• tim: @Twiggy Your comments explaining your variable names are quite good, but it would be even better to just simplify your code. sx for example could be called mapSizeX. And x and y don't seem to be needed as fields at all, so they shouldn't be declared as fields, but as loop variables. I'll still revert your edit, see What you may and may not do after receiving answers