Need help with naming a class!

General Discussion
1

Hi

I am making a serializable class that contains a name and a collection of grid cells that I can build stuff on them.

An example use case is to build a farm grid for the scene, I use it to mark the areas that can be planted or used for placing animal shelters.

This is the class I have created.

/**
 * Defines areas on the grid which we can build on them (i.e place objects).
 *
 * @author Ali-RS
 */
public class GridBuildingArea implements Serializable {

    static final long serialVersionUID = 1L;

    private String name;
    private Grid grid;
    // Keep track of cells by the cell ID
    private final Map<Long, Vec3i> buildableCells = new HashMap<>();

    public GridBuildingArea(String name, Grid grid) {
        this.name = name;
        this.grid = grid;
    }

    public String getName() {
        return name;
    }

    public void setName(String name) {
        this.name = name;
    }

    public Grid getGrid() {
        return grid;
    }

    public void setGrid(Grid grid) {
        this.grid = grid;
    }

    /**
     * Specify if we can build on the cell.
     */
    public void setBuildable(Vec3i cell, boolean buildable) {
        if (buildable) {
            buildableCells.put(grid.cellToId(cell), cell);
        } else {
            buildableCells.remove(grid.cellToId(cell));
        }
    }

    /**
     * @return true if we can build objects on the specified cell.
     */
    public boolean canBuildOn(Vec3i cell) {
        return buildableCells.containsKey(grid.cellToId(cell));
    }

    /**
     * @return true if we can build objects on the specified cell.
     */
    public boolean canBuildOn(int xCell, int yCell, int zCell) {
        return buildableCells.containsKey(grid.cellToId(xCell, yCell, zCell));
    }

    /**
     * @return an unmodifiable map of cells we can build objects on them.
     */
    public Map<Long, Vec3i> getBuildableCells() {
        return Collections.unmodifiableMap(buildableCells);
    }

    /**
     * Clear all buildable cells.
     */
    public void clear() {
        buildableCells.clear();
    }
}

But I am not satisfied with the class naming so I am making a poll.

What name are you suggesting for this class?

  • GridBuildingArea
  • GridArea
  • GridMap
  • BuildingGrid
  • Something else? plaese post below

0 voters

Regards

2

GridCell

2 Likes
3

I don’t know about your naming but it seems like you don’t even use the value of your map and could probably have just had a Set.

1 Like
4

If it were me, I would probably name the entire plot for the farm a BuildingGrid and would name the many cells it contains BuildingGridCell or BuildingCell and store those in a 2d array in the BuildingGrid.

1 Like
5

I think GridArea or GridAreaManager would be best & having a static subclass called Builder to build grid data like setBuildable(book:Boolean), addCell(), removeCell(), notifyManager(), & a build(), but that may expand code a little bit.

1 Like
6

To me, it’s just a marked grid. Or a set of cells. You are using it for “what areas are buildable?” but it’s distinguishing trait is that it efficiently keeps track of a set of grid cells.

So other than the name field part, it could even be: GridCellSet extends AbstractSet<GridCell> with additional methods for using Vec3i. Or just GridCellSet extends AbstractSet<Vec3i> if you only ever deal with grid cells as Vec3i.

…internally you can always translate back and forth between long and Vec3i to keep a Set<Long> internally.

1 Like
7

Two things I always try to keep in mind when naming:

  • Does the name fully describe the function of the class?
  • Is the name generic enough explain all use cases of the class?

With these things in mind, I have often found where I will name a class something to short that does not fully describe its function. I have also found that I may put something too specific in the name of a class, when it has uses outside of that. Do not be afraid of long class names. Many developers I have worked with want nice short class names, but it always comes to bite them when other developers are trying to understand their code. I just recently named a class ComponentNameComponentRegistry yes it is long, but anyone working in the code base will know exactly what it is, especially since a NamedComponentRegistry also exists. One stores components based on the component name, the other stores them based on an assigned name. Two very different functions.

You will notice that @pspeed’s example of GridCellSet take this philosophy of having a name that is generic but fully explains the class.

1 Like
8

Thanks, guys. :slightly_smiling_face:

I picked the GridCellSet following pspeed suggestion. Also removed the name field as it was redundant because it is the same as the file name.

/**
 * Contains grid cells that we are allowed to build on them.
 *
 * @author Ali-RS
 */
public class GridCellSet extends AbstractSet<GridCell> implements java.io.Serializable {

    private final Grid grid;
    private final Set<Long> ids = new HashSet<>();

    private transient Set<GridCell> cells;

    public GridCellSet(Grid grid) {
        this.grid = grid;
    }

    public Grid getGrid() {
        return grid;
    }

    @Override
    public Iterator<GridCell> iterator() {
        return getCells().iterator();
    }

    @Override
    public int size() {
        return ids.size();
    }

    @Override
    public boolean isEmpty() {
        return ids.isEmpty();
    }

    @Override
    public boolean contains(Object o) {
        return getCells().contains(o);
    }

    public boolean contains(Vector3f worldLocation) {
        return getCells().contains(grid.getContainingCell(worldLocation.x, worldLocation.y, worldLocation.z));
    }

    @Override
    public boolean add(GridCell gridCell) {
        boolean added = getCells().add(gridCell);
        if (added) {
            ids.add(gridCell.getId());
        }

        return added;
    }

    @Override
    public boolean remove(Object o) {
        boolean removed = getCells().remove(o);
        if (removed) {
            removed = ids.remove(((GridCell) o).getId());
        }

        return removed;
    }


    @Override
    public void clear() {
        ids.clear();
        cells = null;
    }

    private Set<GridCell> getCells() {
        if (cells == null) {
            cells = ids.stream().map(id -> grid.getGridCell(grid.idToCell(id))).collect(Collectors.toSet());
        }

        return cells;
    }
}

Only the cell IDs are serialized then after loading I use those IDs to construct the actual grid cells.

Edit:

By the way, I am using the Grid class from SimMath library.

9

Note that with a custom iterator, you could even get rid of this redundant cache.

1 Like
10

Thanks for the hint, fixed it.

public class GridCellSet extends AbstractSet<GridCell> implements java.io.Serializable {

    private final Grid grid;
    private final Set<Long> ids = new HashSet<>();

    public GridCellSet(Grid grid) {
        this.grid = grid;
    }

    public Grid getGrid() {
        return grid;
    }

    @Override
    public Iterator<GridCell> iterator() {
        return new GridCellIterator();
    }

    @Override
    public int size() {
        return ids.size();
    }

    @Override
    public boolean isEmpty() {
        return ids.isEmpty();
    }

    @Override
    public boolean contains(Object o) {
        if (o instanceof GridCell gridCell) {
            return ids.contains(gridCell.getId());
        }

        return false;
    }

    public boolean contains(Vector3f worldLocation) {
        return ids.contains(grid.worldToId(worldLocation.x, worldLocation.y, worldLocation.z));
    }

    @Override
    public boolean add(GridCell gridCell) {
        return ids.add(gridCell.getId());
    }

    @Override
    public boolean remove(Object o) {
        if (o instanceof GridCell gridCell) {
            return ids.remove(gridCell.getId());
        }

        return false;
    }


    @Override
    public void clear() {
        ids.clear();
    }

    private class GridCellIterator implements Iterator<GridCell> {

        private final Iterator<Long> idIterator;
        private final Vec3i vTemp = new Vec3i();

        public GridCellIterator() {
            this.idIterator = ids.iterator();
        }

        @Override
        public boolean hasNext() {
            return idIterator.hasNext();
        }

        @Override
        public GridCell next() {
            Long id = idIterator.next();
            return grid.getGridCell(grid.idToCell(id, vTemp));
        }

        @Override
        public void remove() {
            idIterator.remove();
        }
    }
}
11

As an newbie may I ask :
why double ? Couldn’t a float value plus a scaler do the trick?
Is there any performance impact on AMD GPUs ? (lack of support to double ? )
Under what circumstance should we choose double instead of float ?

12

Did you respond to the wrong thread? This one doesn’t seem to have anything to do with double versus float.

…but note that there are plenty of reasons to do game object locations in double and then translate to float for visualization.

1 Like
13

I see

thanks anyway