Java – Creating a fromJSON Static Factory Constructor

javaobject-oriented

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 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?

  1. Product, e.g. by adding a new field named newFeature, changing it to an array, etc... or
  2. 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:

  1. It can go into an interface, JSONable. Static methods cannot be in an interface
  2. Now you can have a Collection<JSONable> which suddenly does a lot for you.
  3. 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.

Related Topic