Data structure manager implementation

Question

I am a beginner Java developer, and I am self learning Java by developing a real life business desktop application intended for management in repair shops. It is unimaginatively called Repair Shop Software. You can see the whole project on GitHub here. I just wanted an opinion on my data-structure management class DataManager. I have a filing I am doing some sort of anti-pattern int this class but I don't know because I am just a beginner. I use the same principle in my ComboBoxModelManager class.

DataManager class:

public class DataManager
{   
    public static IndexedData notificationsDataTable = new IndexedDataTable();
    public static IndexedData notificationTypesDataTable = new IndexedDataTable();
    public static IndexedData servicesDataTable = new IndexedDataTable();
    public static IndexedData serviceTypesDataTable = new IndexedDataTable();
    public static IndexedData ticketsDataTable = new IndexedDataTable();
    public static IndexedData statusTypesDataTable = new IndexedDataTable();
    public static IndexedData clientsDataTable = new IndexedDataTable();
    public static IndexedData marketingTypesDataTable = new IndexedDataTable();
    public static IndexedData legalEntitiesDataTable = new IndexedDataTable();
    public static IndexedData devicesDataTable = new IndexedDataTable();
    public static IndexedData modelsDataTable = new IndexedDataTable();
    public static IndexedData deviceTypesDataTable = new IndexedDataTable();
    public static IndexedData brandsDataTable = new IndexedDataTable();
    public static IndexedData usersDataTable = new IndexedDataTable();

    private static final HashMap<EntityType, IndexedData> dataTables = new HashMap<EntityType, IndexedData>();

    static
    {
        dataTables.put(EntityType.NOTIFICATION, notificationsDataTable);
        dataTables.put(EntityType.NOTIFICATION_TYPE, notificationTypesDataTable);
        dataTables.put(EntityType.SERVICE, servicesDataTable);
        dataTables.put(EntityType.SERVICE_TYPE, serviceTypesDataTable);
        dataTables.put(EntityType.TICKET, ticketsDataTable);
        dataTables.put(EntityType.STATUS, statusTypesDataTable);
        dataTables.put(EntityType.CLIENT, clientsDataTable);
        dataTables.put(EntityType.MARKETING, marketingTypesDataTable);
        dataTables.put(EntityType.LEGAL_ENTITY, legalEntitiesDataTable);
        dataTables.put(EntityType.DEVICE, devicesDataTable);
        dataTables.put(EntityType.MODEL, modelsDataTable);
        dataTables.put(EntityType.DEVICE_TYPE, deviceTypesDataTable);
        dataTables.put(EntityType.BRAND, brandsDataTable);
        dataTables.put(EntityType.USER, usersDataTable);
    }

    public static IndexedData getDataTable(EntityType entityType)
    {
        return dataTables.get(entityType);
    }

    public static Entity getEntity(EntityType entityType, int id)
    {
        return getDataTable(entityType).getEntity(id);
    }

    public static Entity getEntity(EntityType entityType, String name)
    {
        return getDataTable(entityType).getEntity(name);
    }

    public static int getEntityCounter(EntityType entityType)
    {
        return getDataTable(entityType).getEntityCounter();
    }

    public static HashMap<Integer, Entity>getIdMap(EntityType entityType)
    {
        return getDataTable(entityType).getIdMap();
    }

    public static HashMap<String, Entity> getDisplayNameMap(EntityType entityType)
    {
        return getDataTable(entityType).getDisplayNameMap();
    }

    public static boolean isDataTableEmpty(EntityType entityType)
    {
        return getEntityCounter(entityType) == 0;
    }

    public static boolean idCollision(EntityType entityType, int id)
    {
        return getDataTable(entityType).idCollision(id);
    }

    public static boolean displayNameCollision(EntityType entityType, String displayName)
    {
        return getDataTable(entityType).displayNameCollision(displayName);
    }

    public static void save(Entity newEntity)
    {
        getDataTable(newEntity.getEntityType()).save(newEntity);

        ComboBoxModelManager.updateModel(newEntity);
    }

    public static void delete(Entity newEntity)
    {
        getDataTable(newEntity.getEntityType()).delete(newEntity);
    }

    public static void resetTicketCounter()
    {
        ticketsDataTable.resetEntityCounter();
    }
}

To make things more clearer this is the IndexedDataTable class - it is basically a in-memory representation of a database table.

public class IndexedDataTable implements IndexedData
{
    private int entityCounter = 0;
    private HashMap<Integer, Entity> idMap = new HashMap<Integer, Entity>();
    private HashMap<String, Entity> displayNameMap = new HashMap<String, Entity>();

    @Override
    public int getEntityCounter()
    {
        return entityCounter;
    }

    @Override
    public void resetEntityCounter()
    {
        entityCounter =0;
    }

    @Override
    public HashMap<Integer, Entity> getIdMap()
    {
        return idMap;
    }

    @Override
    public HashMap<String, Entity> getDisplayNameMap()
    {
        return displayNameMap;
    }

    @Override
    public Entity getEntity(int id)
    {
        return idMap.get(id);
    }

    @Override
    public Entity getEntity(String displayName)
    {
        return displayNameMap.get(displayName);
    }

    @Override
    public boolean idCollision(int id)
    {
        return idMap.containsKey(id);
    }

    @Override
    public boolean displayNameCollision(String displayName)
    {
        return displayNameMap.containsKey(displayName);
    }

    @Override
    public void save(Entity newEntity)
    {
        entityCounter++;

        idMap.put(newEntity.getId(), newEntity);

        if(EntityType.hasDisplayName(newEntity))
        {
            displayNameMap.put(newEntity.getDisplayName(), newEntity);
        }

        if(EntityType.makesReferences(newEntity))
        {
            newEntity.createReferences();
        }
    }

    @Override
    public void delete(Entity newEntity)
    {
        idMap.remove(newEntity.getId());

        if(EntityType.hasDisplayName(newEntity))
        {
            displayNameMap.remove(newEntity.getDisplayName());
        }

        if(EntityType.makesReferences(newEntity))
        {
            newEntity.deleteReferences();
        }
    }
}

And also for clarity's sake this is the enumeration class EntityType that is used for polymorphism of this and other classes in my project.

public enum EntityType
{
    NOTIFICATION,
    NOTIFICATION_TYPE,
    SERVICE,
    SERVICE_TYPE,
    TICKET,
    STATUS,
    CLIENT,
    MARKETING,
    LEGAL_ENTITY,
    DEVICE,
    MODEL,
    DEVICE_TYPE,
    BRAND,
    USER;

    public static boolean hasDisplayName(EntityType entityType)
    {
        return !(entityType == NOTIFICATION
              || entityType == SERVICE
              || entityType == TICKET
              || entityType == DEVICE);
    }

    public static boolean hasDisplayName(Entity entity)
    {
        return hasDisplayName(entity.getEntityType());
    }



    public static boolean makesReferences(Entity entity)
    {
        EntityType dataType = entity.getEntityType();

        return (dataType == NOTIFICATION)
            || (dataType == SERVICE)
            || (dataType == TICKET)
            || (dataType == CLIENT)
            || (dataType == DEVICE)
            || (dataType == MODEL);
    }

    public static boolean isAProperty(EntityType dataType)
    {
        return (dataType == NOTIFICATION_TYPE)
            || (dataType == MARKETING)
            || (dataType == STATUS)
            || (dataType == DEVICE_TYPE)
            || (dataType == BRAND);
    }
}

And also if anyone has the time to review my whole project, and give my some advice on what an I am doing wrong, and what a I am doing right, I would be very grateful. Link to my project on GitHub.

PS: I don't really know is this a question for Code Review or Software Engineering.

Answer 1

Here is some general recommendation for your code.

1) When using collections, I suggest that you use the interface instead of the implementation in the left part. It will be easier when refactoring and if you want to change the implementation.

Before

private static final HashMap<EntityType, IndexedData> dataTables = new HashMap<EntityType, IndexedData>();

After

private static final Map<EntityType, IndexedData> dataTables = new HashMap<EntityType, IndexedData>();

2) Instead of using the java.util.HashMap implementation with Enum as a key, I suggest that you use the java.util.EnumMap implementation instead.

Javadoc

Implementation note: All basic operations execute in constant time. They are likely (though not guaranteed) to be faster than their HashMap counterparts.

Example

    private static final Map<EntityType, IndexedData> dataTables = new EnumMap<>(EntityType.class);

3) The constant should be all uppercase with words separated by underscores ("_"); it's a naming convention, not required, but highly recommended.

Before

    private static final HashMap<EntityType, IndexedData> dataTables = new HashMap<EntityType, IndexedData>();

After

    private static final HashMap<EntityType, IndexedData> DATA_TABLES = new HashMap<EntityType, IndexedData>();

4) When comparing Enum with else-if, I suggest that you use a Switch-case instead; since the switch case can be faster than the if-else in some cases

• https://stackoverflow.com/questions/6705955/why-switch-is-faster-than-if
• https://stackoverflow.com/questions/10287700/difference-between-jvms-lookupswitch-and-tableswitch

Also, in my opinion, it makes the code more readable and easier to refactor.

Before

    public static boolean hasDisplayName(EntityType entityType)
    {
        return !(entityType == NOTIFICATION
                || entityType == SERVICE
                || entityType == TICKET
                || entityType == DEVICE);
    }

After

    public static boolean hasDisplayName(EntityType entityType)
    {
        switch (entityType)
        {
            case NOTIFICATION:
            case SERVICE:
            case TICKET:
            case DEVICE:
                return false;

            default:
                return true;
        }
    }