Hi there.
After a couple of weeks reading on this forum I thought it was time for me to do my first post.
I'm currently rereading Code Complete. I think it's 15 years since the last time, and I find that I still can't write code ;-)
Anyway on page 138 in Code Complete you find this coding horror example. (I have removed some of the code)
class Emplyee {
public:
FullName GetName() const;
Address GetAddress() const;
PhoneNumber GetWorkPhone() const;
...
bool IsZipCodeValid( Address address);
...
private:
...
}
What Steve thinks is bad is that the functions are loosely related. Or has he writes "There's no logical connection between employees and routines that check ZIP codes, phone numbers or job classifications"
Ok I totally agree with him. Maybe something like the below example is better.
class ZipCode
{
public:
bool IsValid() const;
...
}
class Address {
public:
ZipCode GetZipCode() const;
...
}
class Employee {
public:
Address GetAddress() const;
...
}
When checking if the zip is valid you would need to do something like this.
employee.GetAddress().GetZipCode().IsValid();
And that is not good regarding to the Law of Demeter ([http://en.wikipedia.org/wiki/Law_of_Demeter][1]).
So if you like to remove two of the three dots, you need to use delegation and a couple of wrapper functions like this.
class ZipCode
{
public:
bool IsValid();
}
class Address {
public:
ZipCode GetZipCode() const;
bool IsZipCodeValid() {return GetZipCode()->IsValid());
}
class Employee {
public:
FullName GetName() const;
Address GetAddress() const;
bool IsZipCodeValid() {return GetAddress()->IsZipCodeValid());
PhoneNumber GetWorkPhone() const;
}
employee.IsZipCodeValid();
But then again you have routines that has no logical connection.
I personally think that all three examples in this post are bad. Is it some other way that I haven't thougt about?
//Daniel
