0

Context :
We have a service that accepts a POST submission for an entity. Let us call that entity E. The entity can have various attributes - A,B,C,D. Let us say attribute A is the primary key for entity E. When the service receives a submission, the submission does not need to be complete i.e. it need not have all attributes present apart from the primary key A. If we already have an object in our datastore corresponding to A, we fetch that object, override the stored attribute values with submission attribute values and persist that again. If we have no object stored corresponding to A, we apply some defaulting logic and apply default values to missing attributes before persisting that object to our data store. So the difference in behavior is that submission may or may not have some attributes present but once that object is stored, all attributes will always be present.

Question : In terms of data modeling, I have two options described below. I want to understand which is the recommended way of modeling such behavior and why. :

OPTION 1: Two interfaces treating Optional as contract : 

 interface EntitySubmission { 
    A getA(); // Since this is a primary key it will always be present 
    Optional<B> getB(); 
    Optional<C> getC(); 
    Optional<D> getD(); 
    }

AND

interface EntityPersisted { 
// No attribute will ever be missing or null 
A getA(); 
B getB(); 
C getC(); 
D getD(); 
}

OPTION 2 : Single unified interface treating Optional as convenience : 

interface Entity { 
A getA(); 
Optional<B> getB(); 
Optional<C> getC(); 
Optional<D> getD(); 
}

NOTE : Please note that the intent of OPTION 1 is to clarify the expectation that no attribute will ever be missing or null for objects of type EntityPersisted.

EDIT : I think the core of this question got lost in the Optional debate. The core of my question is whether I should declare separate interfaces to convey separate expectations(Nullable vs NonNull) or have a single unified interface allowing nullable non-key attributes?

kr.ankit
  • 51
  • 1
  • 5
  • Your question is a statement. So what are you asking. I am also not clear on the differences between the two options as OPTION 1 seems to be OPTION 2 on steroids. – hotzst Jan 24 '17 at 15:56
  • I just edited the question. I want to understand which is the recommended way of modeling such behavior and why. – kr.ankit Jan 24 '17 at 15:57
  • 1
    Using `Optional`s that way should be considered an anti-pattern. `Optional` is **not** an "okay, I can be `null`" phrase. It's a standalone type for quite another purposes coming mostly (?) from the Java 8 Streams API. Just make your `getB()` and so forth to return `B` and so forth respectively. If you want to stress a `null`-value semantics, just use `@Nullable` from JSR-305. – Lyubomyr Shaydariv Jan 24 '17 at 16:00
  • The use-case I describe here is somewhat similar to #1 in http://stackoverflow.com/questions/23454952/uses-for-optional Based on that and other such articles strewn around on the internet, I infer that using Optional as a return type of a public method(interface method in my case) is acceptable. Using Optional as class members/method arguments is not. What am I missing in this interpretation @Lyubomyr Shaydariv? – kr.ankit Jan 24 '17 at 16:14
  • @user2369072 `Optional`s are perfect for fluent method chaining like what Java 8 Streams API does. But not for contracts (return values, parameters, fields, whatever). If you want to emphasize that your values can be null, you should do `@Nullable B getB();`, otherwise you should do `@Nonnull B getB();`. Using `Optional`s and simple references both end up with checking for `isPresent` or `== null`. The latter is cheaper and clearly declares the contract. – Lyubomyr Shaydariv Jan 24 '17 at 16:26
  • `interface Entity { @Nonnull A getA(); @Nullable B getB(); ... @Nullable D getD(); }` -- exactly the same contract what your option #2 declares. If you take a look at the `Optional` methods closer, you can see that it's mostly about making transformations (`map`, `flatMap`) and obtaining a _possibly_ existing value. It aligns with the Stream API very nice. Imagine if there were no `Optional`s -- would you use a single-element `Collection` to hold a single value or none then checking for `isEmpty()`? Well, _technically_ you can, but it's _not_ what it was designed for. – Lyubomyr Shaydariv Jan 24 '17 at 16:33
  • How would `@Nullable` and `@Nonnull` help you? You can have static analysis even in your IDE until your code runs. For example, my IntelliJ IDEA is configured to use these annotations for nullability analysis, and I see warnings if I accidentally violate a certain nullability contract. Say, may produce NPE: `B b = entity.getB(); b.doMethodCall()` if the `getB()` method is declared as `@Nullable`. Sure, IntelliJ wouldn't complain if you check `b != null` before you invoke `doMethodCall()`. I have decreased the number of NPEs in my code dramatically for years I use JSR-305. – Lyubomyr Shaydariv Jan 24 '17 at 16:40
  • 1
    @LyubomyrShaydariv So everyone else is wrong about use cases for `Optional`? – Axel Jan 24 '17 at 16:53
  • @Axel For cases like this one -- yes. – Lyubomyr Shaydariv Jan 24 '17 at 16:59
  • 1
    @LyubomyrShaydariv Annotations are **not** code. Annotations are to be used as a help for the compiler / IDE / etc. to know what constraints to check or what code to generate; but once the code is built, they are gone. One should **not** use annotations as part of the API contract, since anybody using that API as an external dependency won't know or care what `@NotNull` annotations might have been used during development. – walen Jan 24 '17 at 17:24
  • @user2369072 your question probably belongs to [codereview] instead. – walen Jan 24 '17 at 17:35
  • @walen This is not really true. JSR-305 nullability annotations _are_ `@Retention(RetentionPolicy.RUNTIME)` at least in FindBugs artifacts allowing to get the annotation information both at compile time and runtime. – Lyubomyr Shaydariv Jan 24 '17 at 17:53
  • 1
    I think the core of this question got lost in the Optional debate. The core of my question is whether I should declare separate interfaces to convey separate expectations(Nullable vs NonNull) or have a single unified interface allowing nullable non-key attributes? – kr.ankit Jan 26 '17 at 01:54
  • @user2369072 I think you can have a single unified interface properly annotated with the nullability annotations as I mentioned in my first comment. – Lyubomyr Shaydariv Jan 26 '17 at 02:29
  • 1
    This is a meaningless debate. Some developers think `Optional` is OK as a return type, while others don't. Both have valid reasons, and both say nonsenses, like "using this or that should be considered an anti-pattern". Voting to close, not because this isn't a good question (actually, it is), but because there are as many valid answers as programmers out there. – fps Jan 26 '17 at 02:54

0 Answers0