Java Class Misuse - Best Practices

Question

I'm fairly certain that I am not doing something class related correctly.

I am using a class to create a set of variables (like a javascript object sort of maybe but not really).

I am using it as shown bellow (a basic example)

public class myScript {

    public static void main(String[] args) {

        Client   warehouse          = new Client();
        Contacts warehouseContactsA = new Contacts();
        Contacts warehouseContactsB = new Contacts();

        warehouse.idno = 1;
        warehouse.name = "warehouse";
        warehouse.desc = "I don't exist";

        warehouseContactsA.client_idno = 1;
        warehouseContactsA.email       = "emailAAA@place.com"

        warehouseContactsB.client_idno = 1;
        warehouseContactsB.email       = "emailBBB@place.com"            

        insertIntoDB(warehouse,
                     warehouseContactsA,
                     warehouseContactsB);

    }

    public static void insertIntoDB(Client   warehouse,
                                    Contacts warehouseContactsA,
                                    Contacts warehouseContactsB) {

        // code to insert into database here

    }

    private class Client {
        int      idno;
        String   name;
        String   desc;
    }

    private class Contacts {
        int      client_idno;
        String   email;
    }

}

Is there any reason to not use classes this way and if so is there a simpler way to store/manage data that doesn't require a class?

Answer 1

Creating inner classes is probably going to create pitfalls for you. When you don't define them as static then they require an implicit reference back to the outer class, which your code doesn't need, it will only get in the way and cause obscure errors. Maybe you're doing this so you can compile only one class and avoid having a build script? Simple build scripts that use gradle are trivial (not like the bad old days when we used ant), so that shouldn't be an issue. It would be better to move your persistent entities out into separate files.

What you're doing with database connections and transactions is not clear. Generally it's bad to try to do each insert in its own transaction, if only because each transaction has overhead and it increases the time the inserts need to run. Typically you'd want to process the inserts in batches.

Mainly, though, if you're writing a script, use a scripting language. Groovy could be a good choice here:

• you wouldn't need an outer class for the procedural script part
• you can define multiple public classes in one file
• Groovy includes a groovy.sql api for simplifying JDBC code.

Answer 2

import java.util.Arrays;
import java.util.List;

public final class WarehouseRepository {

    public static void main(String[] args) {

        WarehouseRepository repository = new WarehouseRepository();

        Client warehouse = new Client(1, "warehouse", "I don't exist");
        Contacts warehouseContactsA = new Contacts(1, "emailAAA@place.com");
        Contacts warehouseContactsB = new Contacts(1, "emailBBB@place.com");

        repository.insertIntoDB(warehouse, Arrays.asList(warehouseContactsA, warehouseContactsB));
    }

    public void insertIntoDB(Client warehouse, List<Contacts> contacts) {
        // code to insert into database here
    }
}

final class Client {
    private final int id;
    private final String name;
    private final String desc;

    public Client(int id, String name, String desc) {
        this.id = id;
        this.name = name;
        this.desc = desc;
    }

    public int getId() {
        return id;
    }

    public String getName() {
        return name;
    }

    public String getDesc() {
        return desc;
    }
}

final class Contacts {
    private final int clientName;
    private final String email;

    public Contacts(int clientName, String email) {
        this.clientName = clientName;
        this.email = email;
    }

    public int getClientName() {
        return clientName;
    }

    public String getEmail() {
        return email;
    }
}

Some things to notice:

1. Try to name the classes with their intentions and some Java conventions. For example, a class doing database operations are usually referred as repository
2. Unless required, make classes and variables final.
3. Make fields private, and if they are must have, then make them constructor parameters, instead of public or getter/setters.
4. If there can be multiple contacts associated with client, then it would be great idea to make List<Contact> as field in client.

Answer 3

Yes, that is a good-enough representation.

No, it's not ideal. There are a couple things you can change to improve the situation:

Visibility option 1: make things private with accessors

You can make things more OO-idiomatic by having your "bean" classes (i.e. objects with data storage purpose only, no logic) fields private, and then having public accessors to mask the inner representation:

public class Client {
  private int id;
  public int getId() { return this.id; }
  public void setId(int id) { this.id = id; }
}

Visibility option 2: make your beans immutable

Another option is to make it so that your beans are immutable (so that you can pass them around in a multi-threaded environment nonchalantly) and also guarantee that they are properly initialized and no one writes to them in an illegal state (for example deleting/zeroing the id but not the rest of the data):

public class Client {
  public final int id;
  public Client(int id) { this.id = id; }
}

Finally, if you have things that may or may not be there (such as description "I don't exist"), I recommend using Optional types, rather than just String types.

Answer 4

I would use Map<String,String> for storing attributes. Where I would store String and ints as Strings and parse them back when they needed.

hope it helps