Question

I have noticed something in my code in various projects that seems like code smell to me and something bad to do, but I can't deal with it.

While trying to write "clean code" I tend to over-use private methods in order to make my code easier to read. The problem is that the code is indeed cleaner but it's also more difficult to test (yeah I know I can test private methods...) and in general it seems a bad habit to me.

Here's an example of a class that reads some data from a .csv file and returns a group of customers (another object with various fields and attributes).

public class GroupOfCustomersImporter {
    //... Call fields ....
    public GroupOfCustomersImporter(String filePath) {
        this.filePath = filePath;
        customers = new HashSet<Customer>();
        createCSVReader();
        read();
        constructTTRP_Instance();
    }

    private void createCSVReader() {
        //....
    }

    private void read() {
        //.... Reades the file and initializes the class attributes
    }

    private void readFirstLine(String[] inputLine) {
        //.... Method used by the read() method
    }

    private void readSecondLine(String[] inputLine) {
        //.... Method used by the read() method
    }

    private void readCustomerLine(String[] inputLine) { 
        //.... Method used by the read() method
    }

    private void constructGroupOfCustomers() {
        //this.groupOfCustomers = new GroupOfCustomers(**attributes of the class**);
    }

    public GroupOfCustomers getConstructedGroupOfCustomers() {
        return this.GroupOfCustomers;
    }
}

As you can see the class has only a constructor which calls some private methods to get the job done, I know that's not a good practice in general but I prefer to encapsulate all the functionality in the class instead of making the methods public in which case a client should work this way:

GroupOfCustomersImporter importer = new GroupOfCustomersImporter(filepath)
importer.createCSVReader();
read();
GroupOfCustomer group = constructGoupOfCustomerInstance();

I prefer this because I don't want to put useless lines of code in the client's side code bothering the client class with implementation details.

So, Is this actually a bad habit? If yes, how can I avoid it? Please note that the above is just a simple example. Imagine the same situation happening in something a little bit more complex.

No correct solution

Licensed under: CC-BY-SA with attribution
scroll top