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.
2 Answers 2
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)
As I see it you have the following alternatives:
- The method should either take the arguments required to create the image and return the created object.
- 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 believeigor– igor2016年05月26日 07:51:55 +00:00Commented May 26, 2016 at 7:51
create
at persistence layer or at upper one (service, control,...)?