You'll get into religious wars here, but let me say that I am on the "ignore SRP" side on this one. Yes, you are coupling to a specific Serialization scheme, as correctly pointed out by @amon. But, in this example, you are coupling to a very abstract and standard scheme, which makes it "o.k.". Lots of Java classes have
valueOf(String), and all have
toString(), and the world hasn't ended due to lack of SRP.
SRP exists not because SRP itself a goal, but because SRP lessens the effect of change, an admirable goal. In this example, what is more likely to change?
Product, e.g. by adding a new field named
newFeature, changing it to an array, etc... or
- The JSON specification and library that you use.
In my experience, "#1" is always the right answer. YMMV.
Product changes, and
fromJSON() is a method of
Product, one class changes. And it was going to change anyway, so there's no "extra" change. Also, you could keep the new fields of
Product private and final, increasing data-hiding and thread-safety.
OTOH, if you are using a
ProductSerializer layer, then two classes change. You are adding stuff about
newFeature in multiple places, which violates DRY. It is trickier to keep the new field private and final. It is just plain extra work and tedium for little or no benefit, violating LEAN or YAGNI. See, I can cite principles too. :-)
One more thing: consider making
fromJSON a real instance method, not a static class function. (It still returns a new Product object) Yes, it feels weird, and you may decide against it. But we "just tried it" in one project and it was wonderful. You may need to add a
final static Product.PROTOTYPE (or similar - maybe call it builder?). But, it opens up some good features:
- It can go into an interface,
JSONable. Static methods cannot be in an interface
- Now you can have a
Collection<JSONable> which suddenly does a lot for you.
- You can provide a testing version, a mock version, etc. If there is a class for which @amon is right and you really do want to decouple the logic into a
ProductBuilder, you could call it here.
If you do change JSON libraries, you will have to change a lot of your classes, so be sure to write them in an intelligent way so that the JSON code is abstracted, as it should be, and as all the libraries do. This is easy.
And, if you were serializing to some weird, offbeat, non-standard, very specific, and likely to change format, yes, then I agree with @amon that the code should be separated. Or it you were tracking somebody else's JSON and they kept changing the names of their field, that would be an issue. Or if the parsing is exceedingly complex, involves combining / "joining" a couple of JSON streams, etc.