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.
3
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 namednewFeature, 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.
If 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 ProductBuilder or 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.
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.
5
There’s nothing stopping you from doing this on a practical level, IMO. Yes, technically it is a violation of separation of concerns, but pragmatically speaking it’s no worse than, say, the Active Record pattern, which is widely used and many have no issue with it.
Alternatively, as suggested in comments, you could use a separate builder object to construct your objects:
Product product = new ProductBuilder().setJSON(jspnObject).build ();
(the implementation of ProductBuilder should be obvious)
A third option is the use of a data binding library, e.g. Jackson.
The problem with your Product.fromJSON method is that it couples the Product to a specific serialization scheme. Furthermore, this violates the Single Responsibility Principle since the Product class isn’t only concerned with modelling something, but also with data exchange.
In general, it makes sense to insulate your internal models from external data exchange via a de- and encoding layer. Instead of networkCommunicator.getJSONFromStream(), we want something like connection.getProduct(). Whether this connection uses JSON or XML or YAML is entirely irrelevant for your Product.
YOUR | TRANSLATION | OUTSIDE
MODEL | LAYER | WORLD
| |
.------- decode <---- JSON
v | |
Product | |
`------> encode ----> JSON
| |
If you happen to be doing web development, you’ve probably heard about the MVC (Model-View-Controller) architecture or some variant of it. The Controller receives the external input and translates this to operations on the model. In the above diagram, this corresponds to the decoding step in the translation layer. If a part of the model is being displayed to the outside world, the model is rendered by the View. In the diagram, this is the encoding step.
Such a translation layer is almost never overkill, since the external data representation and your internal model are likely to be different. This is also the perfect place to do validation on the external input.
2