Htydjtk

According to When is Primitive Obsession not a code smell?, I should create a ZipCode object to represent zip code instead of a String object.

However, in my experience, I prefer to see

public class Address{

    public String zipCode;

}

instead of

public class Address{

    public ZipCode zipCode;

}

because I think the latter one requires me to move to the ZipCode class to understand the program.

And I believe I need to move between many class to see the definition if every primitive data fields were replaced by a class, which feels as if suffering from yo-yo problem.

So I would like to move the ZipCode methods into a new class, eg:

old:

public class ZipCode{

    public boolean validate(String zipCode){

    }

}

new:

public class ZipCodeHelper{

    public static boolean validate(String zipCode){

    }

}

so that only the one who needs to validate the zip code would depend on the ZipCodeHelper class .And I found another "benefit" of keep primitive obsession: it keeps the class looks like its serialized form, if any, for example : an address table with string column zipCode.

My question is, is "avoid yoyo problem" (move between class definitions) a reason to allow "primitive obsession"?

The assumption is that you don't need to yo-yo to the ZipCode class to understand the Address class. If ZipCode is well-designed it should be obvious what it does just by reading the Address class.

Programs are not read end-to-end - typically programs are far too complex to make this possible. You cannot keep all the code in a program in your mind at the same time. So we use abstractions and encapsulations to "chunk" the program into meaningful units, so you can look at one part of the program (say the Address class) without having to read all code it depends on.

For example I'm sure you don't yo-yo into reading the source code for String every time you encounter String in code.

Renaming the class from ZipCode to ZipCodeHelper suggest there now is two separate concepts: a zip code and a zip code helper. So twice as complex. And now the type system cannot help you distinguish between an arbitrary string and a valid zip code since they have the same type. This is where "obsession" is appropriate: You are suggesting a more complex and less safe alternative just because you want to avoid a simple wrapper type around a primitive.

Using a primitive is IMHO justified in the cases where there is no validation or other logic depending on this particular type. But as soon as you add any logic, it is much simpler if this logic is encapsulated with the type.

As for serialization I think it sounds like a limitation in the framework you are using. Surely you should be able to serialize a ZipCode to a string or map it to a column in a database.

If can do:

new ZipCode("totally invalid zip code");

And the constructor for ZipCode does:

ZipCodeHelper.validate("totally invalid zip code");

Then you've broken encapsulation, and added a pretty silly dependency to the ZipCode class. If the constructor doesn't call ZipCodeHelper.validate(...) then you have isolated logic in its own island without actually enforcing it. You can create invalid zip codes.

The validate method should be a static method on the ZipCode class. Now the knowledge of a "valid" zip code is bundled together with the ZipCode class. Given that your code examples look like Java, the constructor of ZipCode should throw an exception if an incorrect format is given:

public class ZipCode {

    private String zipCode;



    public ZipCode(string zipCode) {

        if (!validate(zipCode))

            throw new IllegalFormatException("Invalid zip code");



        this.zipCode = zipCode;

    }



    public static bool validate(String zipCode) {

        // logic to check format

    }



    @Override

    public String toString() {

        return zipCode;

    }

}

The constructor checks the format and throws an exception, thereby preventing invalid zip codes from being created, and the static validate method is available to other code so the logic of checking the format is encapsulated in the ZipCode class.

There is no "yo-yo" in this variant of the ZipCode class. It's just called proper Object Oriented Programming.

We are also going to ignore internationalization here, which may necessitate another class called ZipCodeFormat or PostalService (e.g. PostalService.isValidPostalCode(...), PostalService.parsePostalCode(...), etc.).

The ZipCode abstraction could only make sense if your Address class did not also have a TownName property. Otherwise, you have half an abstraction: the zip code designates the town, but these two related bits of information are found in different classes. It doesn't quite make sense.

However, even then, it's still not a correct application (or rather solution to) primitive obsession; which, as I understand it, mainly focuses on two things:

1. Using primitives as the input (or even output) values of a method, especially when a collection of primitives is needed.


2. Classes that grow extra properties over time without ever reconsidering whether some of these should be grouped into a subclass of their own.

Your case is neither. An address is a well-defined concept with clearly necessary properties (street, number, zip, town, state, country). There is little to no reason to break up this data as it has a single responsibility: designate a location on Earth. An address requires all of these fields in order to be meaningful. Half an address is pointless.

This is how you know that you don't need to subdivide any further: breaking it down any further would detract from the functional intention of the Address class. Similarly, you don't need a Name subclass to be used in the Personclass, unless Name (without a person attached) is a meaningful concept in your domain. Which it (usually) isn't. Names are used for identifying people, they usually have no value on their own.