I have a "Product" POJO class in my app.
A Product object can either be created in-app by the user or by parsing a json that comes from the API.
Product has 20+ fields, but Im using only 2 here for clarity
Example:
JSON productJSON = networkCommunicator.getJSONFromStream();
Product product = new Product();
product.setId(productJSON.getInt("id");
product.setName(productJSON.getString("name");
I want to create a fromJSON constructor in the Product class, but I dont know whether it is a good idea, because it couples the Product class to the API implementation of the JSON structure and the names of the keys in the JSON:
class Product {
private int id;
private String name;
public Product(int id, String name) {
this.id = id;
this.name = name;
}
public static Product fromJSON(JSONObject jsonObject) {
return new Product(jsonObject.getInt("id"), jsonObject.getString("name");
}
}
This will make parsing the JSON coming from server much cleaner.
Best Answer
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 havetoString()
, 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 namednewFeature
, changing it to an array, etc... orIn my experience, "#1" is always the right answer. YMMV.
If
Product
changes, andfromJSON()
is a method ofProduct
, one class changes. And it was going to change anyway, so there's no "extra" change. Also, you could keep the new fields ofProduct
private and final, increasing data-hiding and thread-safety.OTOH, if you are using a
ProductBuilder
orProductSerializer
layer, then two classes change. You are adding stuff aboutnewFeature
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 afinal static Product.PROTOTYPE
(or similar - maybe call it builder?). But, it opens up some good features:JSONable
. Static methods cannot be in an interfaceCollection<JSONable>
which suddenly does a lot for you.ProductBuilder
, you could call it here.Caveats:
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.