Page 1 of 2

Why do Data Classes Smell?

Posted: Tue Mar 13, 2007 5:46 am
by Ollie Saunders
Ahhh I'm finally beginning to write some of that good ol' OO PHP at my new job.
Anyway have you ever seen a class like this:

Code: Select all

class Rabbit
{
    public function __construct($age, $weight, $cuteNess)
    {
    	$this->add = $age;
    	$this->weight = $weight;
    	$this->cuteNess = $cuteNess;
    }
}
$flopsy = new Rabbit(1.2, 3.1, 0.86);
echo $flopsy->cuteNess;
I've come to know classes like this as data classes but according to my, oh so faithful, signature links data classes are a bad thing:
Design Smells wrote:Data Class
Description: Classes with fields and getters and setters and nothing else (aka, Data Transfer Objects - DTO)
Refactor: Move in behavior with Move Method
Firstly why is this so? And if so how the hell are you supposed to "move in behaviour".

Back to the real world, I'm using a data class to store a bunch of properties about a video. URL, title, width, height etc. I'm using MVC so obviously the output is determined by a view. Given the fact that the only behaviour in this case is to be rendered to the browser and a view is going to handle that what am I supposed to do? I don't need any behaviour.

Discuss

Posted: Tue Mar 13, 2007 6:41 am
by mikeq
I guess you need to make the rabbit do something

Code: Select all

class Rabbit 
{ 
    public function __construct($age, $weight, $cuteNess) 
    { 
        $this->add = $age; 
        $this->weight = $weight; 
        $this->cuteNess = $cuteNess; 
    }

    public function Run(){
        if ($this->weight < 10){
             print "Hop, hop hop";
        }
        else if ($this->weight >= 10{
             print "Drag fat body around";
        }
    } 
} 
$flopsy = new Rabbit(1.2, 15, 0.86); 
echo $flopsy->cuteNess;
$flopsy->Run();

Posted: Tue Mar 13, 2007 6:43 am
by jmut
Well it is not necessary bad thing I guess.

I take them as Value Objects but rather a collection of values.
It would be the perfect thing to use in view. Put all data necessary in such object...And use data from it within view template or whatever.

Any link saying it is bad?

Posted: Tue Mar 13, 2007 6:57 am
by Ollie Saunders
jmut wrote:Any link saying it is bad?
http://wiki.java.net/bin/view/People/Sm ... en_Classes

Posted: Tue Mar 13, 2007 6:58 am
by Jenk
I've used this many times..

Code: Select all

class ValueObject
{
    private $_data = array();

    public function __get ($name)
    {
        if (isset($this->_data[$name])) return $this->_data[$name]);
        else return null;
    }

    public function __set ($name, $value)
    {
        $this->_data[$name] = $value;
    }
}
which I also abstract for cases where I might want to overload the setter to validate the entry, or for the getter to throw an exception, or similar.

Posted: Tue Mar 13, 2007 7:41 am
by Begby
I code classes like this a lot. Yeah you can use an array, but its not a fixed structure and since PHP is not strongly typed a class makes it harder to mess up. Also, invariably at some point I'll want to add in some sort of exceptions or datachecking or other some such thing.

Another thing that is nice about it is when you use an IDE with intellisense, it is oh so much easier to have it pop up with a property name than trying to remember what key names are supposed to go in an array.

Posted: Tue Mar 13, 2007 8:10 am
by Maugrim_The_Reaper
I wouldn't necessarily call it a bad smell depending on the context - is it a Transfer Object, Value Object or Data Object? If it's a transfer object then it may be a valid refactoring (Introduce Parameter Object) for allowing easier transfer of data between methods and across classes. In that sense it just reduces the length of parameter lists - not a bad smell. If that's not the case then it may be a bad smell, though the data seems pretty well related since the object represents a single entity, the Rabbit ;).

It's not a Value Object per se since there is a setter.

Posted: Tue Mar 13, 2007 5:37 pm
by Christopher
ole, I really don't see anything wrong with your Rabbit class. It is a classic Value Object. I should note that there are patterns like the Transfer Object which are specific types of Value Objects. But they are Value Objects. I've never heard of a Data Object (and you don't mean DAO obviously).

It is also interesting that you sense a bad smell here -- for there could be one. I don't know where you got that "Refactor: Move in behavior with Move Method" nonsense but it is just wrong. Perhaps they meant Move Property, but that would imply that the properties are either not related or could reside in another class. That is possible, but I don't see it. That refactoring step you found is doubly strange because most often in refactoring you are moving data to/into an object. Hmmmm...

Remember that objects are first data containers, and secondly methods that act upon that data. As programmers we stare a the class code all day long and forget that objects are data. A Value Object, like the one you presented, is very nice in that it has a clear interface to initialize it (the constructor) and the values are easily accessible. You can pass it around and other classes can inherit it that can be polymorphic.

Posted: Tue Mar 13, 2007 5:51 pm
by RobertGonzalez
I think your class represents what it does well. I have used (and continue to use) classes like that regularly. Exactly why do you think it is wrong (other than what you pointed us to already)?

Re: Why do Data Classes Smell?

Posted: Tue Mar 13, 2007 10:06 pm
by AKA Panama Jack
ole wrote:I've come to know classes like this as data classes but according to my, oh so faithful, signature links data classes are a bad thing
For one thing it is a waste of resources to create a class like that. All you are doing is creating a class to hold variables. It's redundant and uses about 3 to 4 times the memory compared to using a normal variable. Plus there is extra CPU overhead used in creating a dummy class like that just to hold variable data.

Think of it like this.

You have a glass containing water. And you decide you need to put that glass inside another glass to make the water more easily transported.

In other words you are just doing something silly.

Re: Why do Data Classes Smell?

Posted: Tue Mar 13, 2007 10:44 pm
by Christopher
AKA Panama Jack wrote:For one thing it is a waste of resources to create a class like that. All you are doing is creating a class to hold variables.
Well if you don't like wasting resources -- don't use PHP. You realize that allocating an array with three integers like the values in ole's object would take 200-300 bytes. The same Value Object would probably be 400-500 bytes. Objects obviously increase in size when you add more to the class -- but you are adding more functionality.
AKA Panama Jack wrote:It's redundant and uses about 3 to 4 times the memory compared to using a normal variable. Plus there is extra CPU overhead used in creating a dummy class like that just to hold variable data.
Again it can be less than %50 more or 3-4 times more depending on the class. But remember, you can't add methods to an array later or extend it...
AKA Panama Jack wrote:Think of it like this.

You have a glass containing water. And you decide you need to put that glass inside another glass to make the water more easily transported.

In other words you are just doing something silly.
A glass in a glass?!? Huh?!?

Re: Why do Data Classes Smell?

Posted: Wed Mar 14, 2007 3:06 am
by jmut
AKA Panama Jack wrote:
ole wrote:I've come to know classes like this as data classes but according to my, oh so faithful, signature links data classes are a bad thing
For one thing it is a waste of resources to create a class like that. All you are doing is creating a class to hold variables. It's redundant and uses about 3 to 4 times the memory compared to using a normal variable. Plus there is extra CPU overhead used in creating a dummy class like that just to hold variable data.

Think of it like this.

You have a glass containing water. And you decide you need to put that glass inside another glass to make the water more easily transported.

In other words you are just doing something silly.
Do not agree at all. As we all know ...
"preoptimization is the root of all evil" . If you don't know it, read about it.
and this is exactly what you are trying to promote here.

Posted: Wed Mar 14, 2007 4:44 am
by Ollie Saunders
jmut wrote:Do not agree at all. As we all know ...
"preoptimization is the root of all evil" . If you don't know it, read about it.
and this is exactly what you are trying to promote here.
Absolutely true. The performance argument doesn't wash with me.
Everah wrote:Exactly why do you think it is wrong (other than what you pointed us to already)?
Just that really. I'm asking this question because I pretty much agree with all of you in that I can't see anything wrong with it either.

Re: Why do Data Classes Smell?

Posted: Wed Mar 14, 2007 5:05 am
by Xoligy
AKA Panama Jack wrote:
ole wrote:I've come to know classes like this as data classes but according to my, oh so faithful, signature links data classes are a bad thing
For one thing it is a waste of resources to create a class like that. All you are doing is creating a class to hold variables. It's redundant and uses about 3 to 4 times the memory compared to using a normal variable. Plus there is extra CPU overhead used in creating a dummy class like that just to hold variable data.

Think of it like this.

You have a glass containing water. And you decide you need to put that glass inside another glass to make the water more easily transported.

In other words you are just doing something silly.
I've got to say I agree with you. You just seem to be over-complicating things using a class. Unless you intend to add methods to it, an array seems more appropriate. Although having said that, Javascript treats all arrays as objects, which can be handy.

Posted: Wed Mar 14, 2007 9:19 am
by Ollie Saunders
How is it overcomplicating things?
With an array you have to remember what the keys are. You can add new keys (through typos) without be notified of your error. This would be pointless:

Code: Select all

class Rabbit
{
    public $age, $weight, $cuteNess;
}
because its just mimicing an array with an object. You might as well:

Code: Select all

$obj = new stdClass();
but this, for me...

Code: Select all

class Rabbit
{
    public function __construct($age, $weight, $cuteNess)
    {
        $this->add = $age;
        $this->weight = $weight;
        $this->cuteNess = $cuteNess;
    }
}
...represents an improvement in extendibility and accidental error reduction.