1

We have this bean with two properties:

ContainerImage
    + name (final, not null)
    + id (null)

Then there is this method:

ContainerImage create(ContainerImage containerImage);

It receives ContainerImage with name and without the id; and returns new ContainerImage with id populated, as result from the underlaying operation.

For me, this has bad smell; because input simply does not use the id, so why would I pass it? Also, it does not change the incoming object, which is also fine, but someone might add the change, which I don't want.

My proposal is to have an read-only interface for ContainerImage that does not takes the id, that will be used for input argument, e.g.:

ContainerImageData
    + name

Does this make sense for you?

p.s. I am talking Java now, but the question is rather general.

EDIT

Method create creates docker images, so it is a method that works with docker remote api, plus it may do some few things aside. But I would like to think about this question without knowing what it really does, just from the API side.

  • Is method create at persistence layer or at upper one (service, control,...)? – Laiv May 25 '16 at 10:57
  • It creates docker image. So it is kind of persistence. Let me edit the question. – igor May 25 '16 at 10:59
  • Generally speaking, it looks like raw content and registered/id'd content ought to be different types. – Erik Eidt May 25 '16 at 15:00
1

There's nothing wrong on using POJOs or TOs (transaction objects) in order to inform an specific set of fields. It also helps to decouple layers.

It's common at upper layers as control or business for several reasons:

  • These layers doesn't want to expose the real structure of the data.
  • These layers wants to provide specific/easy ways to inform the data.
  • Those who consume these layers may or may not to know how real data structure (entity) is.
  • POJOs do these layers agnostic to the real data structure. This is useful when data source is out of your control or maintenance, so rather don't be too dependent on such data. POJOs acts like a defensive line that protect your business from unwanted changes.

So ContainerImageData which purpose is to provide an specific set of fields needed to create an entry, fits well into TO definition and usages.

Making few changes, in your case, I would go for:

ContainerImage create(AddContainerImageTO to){
     ContainerImage container = new ContainerImage(to.getName());
     <dao>.create(container);
     return container;
}

Just to guarantee that AddContainerImageTO has the right state, I would provide it with a specific constructor and just getters.

AddContainerImageTO{          
      AddContainerImageTO(String name){...}
      String getName(){...}
}
//Note: I didn't provide setters for a good reason ;-)

Or has many constructors as possible valid states were allowed. (Unless there were too many, in such case, builder pattern would prevail)

  • this is very close to my thinking – igor May 26 '16 at 7:53
  • 1
    Yes, you were doing fine so far (IMO). I just added some tips like the way to generate the POJO :-) and its advantages. – Laiv May 26 '16 at 8:35
0

As I see it you have the following alternatives:

  1. The method should either take the arguments required to create the image and return the created object.
  2. The method should be void CreateImage(image) and simply attach the generated ID to the passed object.

The second alternative might not be 100% good design since we have had to create an object which is in an inconsistent state until the create method have been called. But it's a small sacrifice which many seem to accept compared to the alternative (it's common when using of ORM libraries).

because input simply does not use the id, so why would I pass it

Because it's not often that methods use all properties of an object. If we only should pass relevant properties than we would have a mess in the source code.

Another good thing with passing objects instead of their properties is encapsulation. When we pass objects we can be certain that they represent a valid state, since objects should protect their state (if we follow OOP principles). By passing their properties as arguments you do not get that guarantee.

  • ID here is not just a simple property. It is also indicator that image was already created. Hence passing an instance with this ID set, it's kind of an logical error, i believe – igor May 26 '16 at 7:51

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.