Is separating most classes into data field only class and method only classes (if possible) a good or an anti-pattern?

Question

For example, a class usually have class members and methods, eg:

public class Cat{
    private String name;
    private int weight;
    private Image image;

    public void printInfo(){
        System.out.println("Name:"+this.name+",weight:"+this.weight);
    }

    public void draw(){
        //some draw code which uses this.image
    }
}

But after reading about Single responsibility principle and Open closed principle, I prefer separate a class into DTO and helper class with static methods only, eg:

public class CatData{
    public String name;
    public int weight;
    public Image image;
}

public class CatMethods{
    public static void printInfo(Cat cat){
        System.out.println("Name:"+cat.name+",weight:"+cat.weight);
    }

    public static void draw(Cat cat){
        //some draw code which uses cat.image
    }
}

I think it fits Single responsibility principle because now the responsibility of CatData is to keep data only, doesn't care about the methods (also for CatMethods). And it also fits the open closed principle because adding new methods doesn't need to change the CatData class.

My question is, is it a good or an anti-pattern?

Answer 1

You have shown two extremes ("everything private and all (maybe unrelated) methods in one object" vs. "everything public and no method inside the object"). IMHO good OO modeling is none of them, the sweet spot is somewhere in the middle.

One litmus test of what methods or logic belongs into a class, and what belongs outside, is to look at the dependencies the methods will introduce. Methods which don't introduce additional dependencies are fine, as long as they fit well to the abstraction of the given object. Methods which do require additional, external dependencies (like a drawing library or an I/O library) are seldom a good fit. Even if you would make the dependencies vanish by using dependency injection, I still would think twice if placing such methods inside the domain class is really necessary.

So neither you should make every member public, nor do you need to implement methods for every operation on an object inside the class. Here is an alternative suggestion:

public class Cat{
    private String name;
    private int weight;
    private Image image;

    public String toString(){
        return "Name:"+this.name+",weight:"+this.weight;
    }
    public Image getImage(){
        return image;
    }
}

Now the Cat object provides enough logic to let surrounding code easily implement printInfo and draw, without exposing all attributes in public. The right place for these two methods is most probably not a god class CatMethods (since printInfo and draw are most probably different concerns, so I think it is very unlikely they belong into the same class).

I can imagine a CatDrawingController which implements draw (and maybe uses dependency injection for getting a Canvas object). I can also imagine another class which implements some console output and uses toString (so printInfo may become obsolete in this context). But for making sensible decisions on this, one need to know the context and how the Cat class will actually be used.

That is actually the way how I interpreted Fowler's Anemic Domain Model critics - for generally reusable logic (without external dependencies) the domain classes themselves are a good place, so they should be used for that. But that does not mean to implement any logic there, quite the opposite.

Note also the example above leaves still room for making a decision about (im)mutability. If the Cat class will not expose any setters, and Image is immutable itself, this design will allow to make Cat immutable (which the DTO approach won't). But if you think immutability is not required or not helpful for your case, you can also go into that direction.

Answer 2

Late answer but I can't resist.

Is X most classes into Y good or an anti-pattern?

In most cases, most rules, applied without thinking, will mostly go horribly wrong (including this one).

Let me tell you a story about the birth of an object amid the chaos of some down right, quick and dirty, procedural code that happened, not by design, but out of desperation.

My intern and I are pair programming to quickly create some throw away code to scrape a webpage. We have absolutely no reason to expect this code will live long, so we're just banging out something that works. We grab the whole page as a string and chop out the stuff we need in the most amazingly brittle way you could imagine. Don't judge. It works.

Now while doing this I created some static methods to do the chopping. My intern created a DTO class that was very much like your CatData.

When I first looked at the DTO it bugged me. The years of damage Java has done to my brain made me recoil at the public fields. But we were working in C#. C# has no need of premature getters and setters to preserve your right to make the data immutable, or encapsulated later. Without changing the interface you can add them whenever you like. Maybe just so you can set a breakpoint. All without telling your clients a thing about it. Yea C#. Boo Java.

So I held my tongue. I watched as he used my static methods to initialize this thing before using it. We had about 14 of them. It was, ugly, but we had no reason to care.

Then we needed it in other places. We found ourselves wanting to copy and paste the code. 14 lines of initialization being flung around. It was starting to get painful. He hesitated and asked me for ideas.

Reluctantly I asked, "would you consider an object?"

He looked back at his DTO and screwed up his face in confusion. "It is an object".

"I mean a real object"

"Huh?"

"Let me show you something. You decide if it's useful"

I chose a new name and quickly whipped up something that looked a little like this:

public class Cat{
    Cat(string catPage) {
        this.catPage = catPage
    }
    private readonly string catPage;

    public string name() { return chop("name prefix", "name suffix"); }
    public string weight() { return chop("weight prefix", "weight suffix"); }
    public string image() { return chop("image prefix", "image suffix"); }

    private string chop(string prefix, string suffix) {
        int start = catPage.indexOf(prefix) + prefix.Length;
        int end = catPage.indexOf(suffix);
        int length = end - start;
        return catPage.Substring(start, length);
    }
}

This did nothing the static methods weren't already doing. But now I'd sucked the 14 static methods into a class where they could be alone with the data they worked on.

I didn't force my intern to use it. I just offered it and let him decide if he wanted to stick with the static methods. I went home thinking he'd probably stick to what he already had working. The next day I found he was using the object in a bunch of places. It decluttered the rest of the code which was still ugly and procedural but this bit of complexity was now hidden from us behind an object. It was a little better.

Now sure every time you access this it's doing a fair bit of work. A DTO is a nice fast cached value. I worried about that but realized I could add the caching if we ever needed without touching any of the using code. So I'm not going to bother until we care.

Am I saying you should always stick to OO objects over DTO's? No. DTO's shine when you need to cross a boundary that keeps you from moving methods. DTO's have their place.

But so do OO objects. Learn how to use both tools. Learn what each costs. Learn to let the problem, the situation, and the intern decide. Dogma is not your friend here.

---

Since my answer is already ridiculously long let me disabuse you of some misconceptions with a review of your code.

For example, a class usually have class members and methods, eg:

public class Cat{
    private String name;
    private int weight;
    private Image image;

    public void printInfo(){
        System.out.println("Name:"+this.name+",weight:"+this.weight);
    }

    public void draw(){
        //some draw code which uses this.image
    }
}

Where's your constructor? This isn't showing me enough to know if it's useful.

But after reading about Single responsibility principle and Open closed principle, I prefer separate a class into DTO and helper class with static methods only, eg:

public class CatData{
    public String name;
    public int weight;
    public Image image;
}

public class CatMethods{
    public static void printInfo(Cat cat){
        System.out.println("Name:"+cat.name+",weight:"+cat.weight);
    }

    public static void draw(Cat cat){
        //some draw code which uses cat.image
    }
}

I think it fits Single responsibility principle because now the responsibility of CatData is to keep data only, doesn't care about the methods (also for CatMethods).

You can do many silly things in the name of the Single Responsibility Principle. I could argue that Cat Strings and Cat ints should be separated. That drawing methods and Images must all have their own class. That your running program is a single responsibility so you should only have one class. :P

To me, the best way to follow the Single Responsibility Principle is to find a good abstraction that lets you put complexity in a box so you can hide it. If you can give it a good name that keeps people from being surprised by what they find when they look inside you've followed it fairly well. Expecting it to dictate more decisions then that is asking for trouble. Honestly, both of your code listings do that so I don't see why SRP matters here.

And it also fits the open closed principle because adding new methods doesn't need to change the CatData class.

Well no. The open close principle isn't about adding new methods. It's about being able to change the implementation of old methods and having to edit nothing. Nothing that uses you and not your old methods. Instead you write some new code somewhere else. Some form of polymorphism will do that nicely. Don't see that here.

My question is, is it a good or an anti-pattern?

Well hell how should I know? Look, doing it either way has benefits and costs. When you separate code from data you can change either without having to recompile the other. Maybe that is critically important to you. Maybe it just makes your code pointlessly complicated.

If it makes you feel better you aren't that far from something Martin Fowler calls a parameter object. You don't have to only take primitives into your object.

What I would like you to do is develop a sense for how to do your separation, or not, in either coding style. Because believe it or not you're not being forced to choose a style. You just have to live with your choice.

Answer 3

You have stumbled across a topic of debate that has been creating argument amongst developers for well over a decade. In 2003, Martin Fowler coined the phrase "Anaemic Domain Model" (ADM) to describe this separation of data and functionality. He - and others that agree with him - argue that "Rich Domain Models" (mixing data and functionality) is "proper OO", and that the ADM approach is a non-OO anti-pattern.

There have always been those that dismiss this argument and that side of the argument has grown louder and bolder in recent years with the adoption by more developers of functional development techniques. That approach actively encourages the separation of data and function concerns. The data should be immutable as much as possible, so the encapsulation of mutable state becomes a non-concern. There is no benefit to attaching functions directly to that data in such situations. Whether is then "not OO" or not is of absolutely no interest to such folk.

Regardless of which side of the fence you sit on (I sit very firmly on the "Martin Fowler is talking a load of old tosh" side BTW), your use of static methods for printInfo and draw is near-universally frowned upon. Static methods are hard to mock when writing unit tests. So if they have side effects (such as printing or drawing to some screen or other device), they shouldn't be static, or should have the output location passed in as a parameter.

So you could have an interface:

public interface CatMethods {
    void printInfo(Cat cat);
    void draw(Cat cat);
}

And an implementation that gets injected into the rest of your system at runtime (with other implementations being used for testing):

internal class CatMethodsForScreen implements CatMethods {
    public void printInfo(Cat cat) {
        System.out.println("Name:"+cat.name+",weight:"+cat.weight);
    }

    public void draw(Cat cat) {
        //some draw code which uses cat.image
    }
}

Or add extra parameters to remove the side-effects from those methods:

public static class CatMethods {
    public static void printInfo(Cat cat, OutputHandler output) {
        output.println("Name:"+cat.name+",weight:"+cat.weight);
    }

    public static void draw(Cat cat, Canvas canvas) {
        //some draw code which uses cat.image and draws it on canvas
    }
}

Answer 4

DTOs -- Data Transport Objects

are useful for just that. If you are going to shunt data between programs or systems than DTOs are desirable as they provide a manageable sub-set of an object which is concerned only with data structure and formatting. The advantage being that you do not have to synchronise updates to complex methods over several systems (as long as the underlying data does not change).

The whole point of OO is to bind data and the code acting on that data closely together. Separating a logical Object into separate classes is usually a bad idea.

Answer 5

Is it a good pattern?

You're probably going to get different answers here because people follow different schools of thought. So before I even begin: This answer is according to object-oriented programming as it is described by Robert C. Martin in the book "Clean Code".

---

"True" OOP

As far as I'm aware, there's 2 interpretations of OOP. For most people, it boils down to "an object is a class".

However, I found the other school of thought more helpful: "An object is something that does something". If you're working with classes, each class would be one of two things: a "data-holder" or an object. Data-holders hold data (duh), objects do stuff. That does not mean that a data-holder can't have methods! Think of a list: A list doesn't really do anything, but it has methods, to enforce the way it holds the data.

Data holders

You Cat is a prime example of a data holder. Sure, outside of your program a cat is something that does something, but inside your program, a Cat is a String name, an int weight and an Image image.

That data holders don't do anything also doesn't mean that they don't contain business logic! If your Cat class has a constructor requiring name, weight and image, you've successfully encapsulated the business rule that every cat has those. If you can change the weight after a Cat class has been created, that's another business rule encapsulated. These are very basic things, but that just means that's it's very important to not get them wrong - and this way, you ensure that it's impossible to get that wrong.

Objects

Objects do something. If you want Clean* objects, we can change that to "Objects do one thing".

Printing the cat info is the job of an object. For example, you could call this object "CatInfoLogger", with a public method Log(Cat). That is all we need to know from the outside: This object logs a cat's info and to do so, it needs a Cat.

On the inside, this object would have references to whatever it needs to fulfill its single responsibility of logging cats. In this case, that's only a reference to System for System.out.println, but in most cases, it will be private references to other objects. If the logic for formatting the output before printing it becomes too complex, CatInfoLogger can just get a reference to a new object CatInfoFormatter. If later on, every log needs to also be written to a file, you can add a CatToFileLogger object which does that.

Data holders vs objects - summary

Now, why would you do all that? Because now changing a class changes only one thing (the way a cat is logged in the example). If you're changing an object, you're only changing the way a certain action is done; if you change a data holder, you only change what data is held and/or in which way it is held.

Changing data might mean the way something is done has to change, too, but that change won't happen until you make it happen yourself by changing the responsible object.

Overkill?

This solution might seem a little overkill. Let me be frank: It is. If your whole program is only as big as the example, don't do any of the above - just choose either of your proposed ways with a coin toss. There is no danger of confusing other code if there is no other code.

However, any "serious" (= more than a script or 2) software is probably big enough that it would profit from a structure like this.

---

Answer

Is separating most classes into DTO and helper classes [...] a good or an anti-pattern?

Turning "most classes" (without any further qualification) into DTOs/data holders, is not an anti pattern, because it's actually not any pattern - it's more or less random.

Keeping data and objects apart however is seen as a good way to keep your code Clean*. Sometimes you will only need a flat, "anemic" DTO and that's it. Other times, you need methods to enforce the way the data is held. Just don't put your program's functionality with the data.

---

*That's "Clean" with a capital C like the book title, because - remember the first paragraph - this is about one school of thought, while there are also others.

Answer 6

Many design patterns and principles conflict, and ultimately only your judgement as a good programmer will help you decide which patterns should apply for a specific problem.

In my opinion, the Do The Simplest Thing That Could Possibly Work principle should win by default unless you have a strong reason to make the design more complicated. So in your specific example, I would opt for the first design, which is a more traditional object-oriented approach with data and functions together in the same class.

Here are a couple questions you could ask yourself about the application:

• Will I ever need to provide an alternate way of rendering a Cat image? If so, will inheritance not be a convenient way to do that?
• Does my Cat class need to inherit from another class or satisfy an interface?

Answering yes to any of these might lead you to a more complex design with separate DTO and functional classes.

Answer 7

I think an important consideration, and a good reason for separating data objects from method objects that has not been discussed here, is the situation where an application needs to retrieve and process a large volume of data. If the data object contains all of the business logic code associated with that data, a separate instance of all of that code has to be loaded into memory along with each instance of the data. This may not be an issue on server based apps with copious cores and RAM, but it is an issue on mobile apps. This may not be "true OOP", but in the case of processing large volumes of data, separating the data object from associated business logic code would make an app significantly faster and more machine efficient.