How to refactor a large class with many methods that have single responsibility?

Question

Searching for design patterns and better code optimization mostly pops up articles on inheritance and relationships between classes and tools on how to create class diagrams. I would like a little insight on how to, say split, a big class. Am working on a java program with a class that has crossed 1600 lines of code and about 20 methods. What it does is query for data from a data source and generate huge text file.

Now there are lots of data modification logic(for each logic I have created small methods) that goes on to queried data like:

-check data dates of users and perform changes
-append strings
-logic to check if a record needs to be ignored

and storing filtered data in Collection and generating text file. I don't think creating separate class for each small logic is good idea. Perhaps I can create separate utility class and stuff static methods into it? I can't post code for confidentiality issues. If someone can shed a little light to get me thinking in right direction that would be great. Thanks.

Answer 1

The best thing you can do is forget what the code does, and think about what it is.

From the description you gave, you are "still" thinking about what your code is supposed to do. It queries a data source, checks dates, appends strings, filters, etc. This is not wrong for a functional or procedural approach, but not ideal for object-oriented code.

Try a little bit to forget what it supposed to do, and think about what "things" it is about. Are those things Accounts, Users, Machines, VirtualServers, etc. What is that huge text file? Is it a UsageReport, a BalanceSheet, etc.

Once you got the "things", you can start thinking about what "responsibilities" you need to assign each of them. Again, "querying the database" is not a responsibility. You have to do something with the data you get, for example generating a UsageReport, that is a responsibility.

That is OO in a nutshell. A few more pointers: Try to avoid utilities, try to avoid getters, try to avoid setters even more. Try to avoid Services, Processor, Handler, Renderer and similar classes. Those are not "things", but usually procedures in disguise.

Answer 2

Perhaps you should consider whether your initial premise is correct. The single responsibility principle states that a class should only have one reason to change.

In the case of the class you're describing, there are multiple responsibilities: querying a data source, filtering, string manipulation (presentation?), text file generation.

Each of these should be separated into its own class. This decouples the concerns and provides smaller more manageable code.

Answer 3

SRP (Single Responsibility Principle) suggests that we should have only one reason to change a class. i.e. a class should have only one single responsibility. So one class should do only one thing. So in an ideal object oriented environment, we would have classes for each of our work.

Ex:

Assume you are building a Calculator app with basic math support. Assume you have to implement Summing two numbers feature. In ideal way, you would have to do something like this.

public interface NumberMath{
    public float value();
}

public class Sum extends NumberMath{
    private float num1;
    private float num2;

    public Sum(float num1, float num2){
        this.num1 = num1;
        this.num2 = num2;
    }

    @Overrride
    public float value(){
        return num1 + num2;
    }
}

I think you already found that it will make our code much verbose. So practically no one will follow object orientation (or SRP) in ideal way. Instead most of us would prefer one class containing methods for sum, subtract, division, multiplication etc. It makes our code less verbose.

But in your case 1600 lines of code with 20 methods for 1 class can't be good.

The best ways you can decompose your huge single class into multiple simple classes

• Identify independent methods.
• Combine similar small methods(behaviors) into a single class with a proper scope for that class, and a name that would properly define that scope.
• Try to completely avoid code duplication by adhering into method reuse instead.

Again it is somewhat difficult to provide a very specific answer for your problem since details are limited. Anyway hope this helps. :)