search

oasp / oasp4j

The Open Application Standard Platform for Java
Apache License 2.0
60 stars 303 forks source link

Replace Long with primitive long in UC-Methods #149

Closed maihacke closed 8 years ago

maihacke commented 9 years ago

In UC-find-Method passing Null-Values for IDs does not make sense. We should consider using primitive long instead of object form Long here.

hohwille commented 9 years ago

I do not really see a real benefit from this. Will cause even more boxing and unboxing for nothing. Just my few cents...

maybeec commented 9 years ago

I am with @maihacke as we are also talking about @NotNull annotations to avoid nullpointer exceptions and get warned by the compliler very early. So why not being more clear here and use primitives where null values will not occur?

hohwille commented 9 years ago

Do you then propose to also change that in DAOs and Services to be consistent? Be also aware that we are using a generic for PK in GenericDao and Java prevents to fill in primitive types for generics. I do not mind to change the methods as you propose but I also do not see much value in this improvement. If you want to prioritize then feel free to contribute :)

maybeec commented 9 years ago

Ok I see the problem of consistency. But nevertheless, if we want to provide generic implementations this is a fallback we have to take. However, restricting the interface on logic as well as on service layer would ensure correct behavior due to data sovereignty of components for any parameter passed to the system.

Otherwise you will end up with odd trivial not null checks and some exception handling to cover this cases... nothing I wanna do.

hohwille commented 9 years ago

I think nobody will (or shall) implement not null checks in these cases but you might end up with NPEs what is pain. So lets change the Services and the Use-Cases to use primitive long.

hohwille commented 8 years ago

Indeed this missed 1.5.0. Will be merged into development line next week and come with the next release.

hohwille commented 8 years ago

PR has been mergend. Thanks to @fawinter and @sesslinger.