Is this a good way to implement a tile map from a text file?

I'm working on a small final fantasy type game. I have this for a tile map. I was just wondering if this is a good way to do this or if anyone has any suggestions, I would love to read them.

```
import com.stardust.main.gfx.Assets;

import java.awt.Graphics;
import java.awt.image.BufferedImage;
import java.io.BufferedReader;
import java.io.FileReader;
import java.io.IOException;

public class TileMap
{
//placeholder for each tile
BufferedImage[] tiles = new BufferedImage[37];

//array of tile types
int[][] map = new int[32][32];
//position coordinates
public static int posX, posY;
//Edge of map Cooridinates
public static int sx, sy;
public String fileName;

public TileMap(int posX, int posY, String fileName) throws IOException
{
this.fileName = fileName;
int x = 0, y = 0;
this.posX = posX;
this.posY = posY;
this.sx = map.length * 32;
this.sy = map.length * 32;
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);
map[x][y] = str_int;
y += 1;
}
x += 1;
y = 0;
}

in.close();
}

public void update()
{
}

//Game.mapX and Game.mapY are the position variables in the main Game class
public void render(Graphics g)
{
for(int x = 0; x < 32; x++)
{
for(int y = 0; y < 32; y++)
{
int textureType = map[x][y];
setTile();
BufferedImage texture = tiles[textureType];
g.drawImage(texture, posX, posY, null);
posY += 32;
}
posX += 32;
posY = Game.mapY;
}
posX = Game.mapX;
posY = Game.mapY;

}

publ

Single responsibility principle

This class is responsible for one too many things:

• Model a tile map

• Parse a text file

As such it violates the single responsibility principle.
I suggest to move the file parsing code somewhere else.
For example in a TileMapFactory.fromFile method, or TileMapFileParser.

In general, you know something's not right when the constructor is too big.
If it has to do IO, which is especially troublesome because of exception handling and resource management, that's really way too much burden for a constructor.

Prefer List instead of arrays

If you don't need specifically an array, then use a List instead:

List tiles = new ArrayList<>();

It will make your life easier. For example, when adding the tiles,
you won't need to worry about correctly incrementing the array indexes:

tiles.add(Assets.grass);
   tiles.add(Assets.dirt);
   // ... and so on

Magic numbers

The number 32 keeps coming up at many places in the code.
Declare it as a static final variable with a meaningful name.

Get rid of unused stuff

For example this method is pointless, remove it:

public void update()
   {
   }

Coding style

Instead of y += 1, use ++y.

StackExchange Code Review Q#69213, answer score: 6