Do I suffer from encapsulation overuse?
- by Florenc
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 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.
