[hibernate-dev] [OGM] Option identity

[Gunnar Morling]

2013/10/21 Emmanuel Bernard <emmanuel at hibernate.org>

> Gunnar has I had a long discussion and disagreement on the behavior of
> Option in Hibernate OGM that we could not resolve. We would like your
> feedback.
>
> I did design Option to represent three key concepts:
>
> - an option family - for example named query
> - an option per se - for example the combination of the named query family
> and the query name
> - an option value - for example the named query value (the HQL)
>
> These notions used to be separated in a previous incarnation but for the
> sake of simplification, we decided to merge option and option value.
>
> So when I designed Option I added the notion of option identifier that
> combined with an option class (the family) would uniquely identify an
> option. This notion is represented by getOptionIdentifier() which is used
> by equals() and hashCode(). The idea is that an Option instance uniquely
> identifies an option and that we could detect when a user tries to define
> the same option several times (with or without different values).
>
> We have two kinds of options:
>
> - generic options like named query that required an additional key to be
> uniquely identified
> - unique options like show_sql where the option family and the option are
> one and the same thing
>
> In the unique option case, a UniqueOption subclass force the
> implementation of getOptionIdentifier so that equals / hashCode are true if
> the same option / option family are requested.
>
> A goal of getOptionIdentifier() was to free the Option implementor from
> thinking too much about the equals() / hashCode implementation and just
> focus on the option identity.
>
> Now Gunnar had a concern when he experimented with unique options
>
>     class ShowSql extends UniqueOption {
>         private String value;
>         public ShowSql(String value) {
>             this.value = value;
>         }
>         public static ShowSql TRUE = new ShowSql("true");
>         public static ShowSql FALSE = new ShowSql("false");
>     }
>
> In this situation, ShowSql.equals(ShowSql.FALSE) == true and that bothers
> him.
>
> So he changed the implementation of Option / UniqueOption to behave like I
> previously explained for generic options but to properly return
> ShowSql.equals(ShowSql.FALSE) == false. To do so, he removed
> getOptionIdentity() and manually ask (Unique)Option implementors to
> implement equals. In his example equals for ShowSql would be based on the
> value.
>
> So we are at an impasse.
>
> I hate his approach as the equals behavior is inconsistent between generic
> options and unique options. It's very confusing for an Option implementor
> to figure out what equals should do. And ice on the cake, the implementor
> has to implement equals / hashCode instead of delegating that to the
> superclass: more code, more opportunities to screw it up.
> To me the fact that ShowSql.equals(ShowSql.FALSE) == true is indeed
> inelegant but AFAIAC is a degenerate case and shifts the identity from
> Option to Option + Option Value which is useless.
>

I don't think it's a degenerated case but a convenient way for working with
options. Compare

boolean showSql;
ShowSqlOption option = optionsContainer.getUnique(ShowSqlOption.class);
showSql = option != null ? option.isShowSql() : false;

vs.

boolean showSql =
ShowSqlOption.TRUE.equals(optionsContainer.getUnique(ShowSqlOption.class));

Personally I'd prefer to work with the latter. Also dialect implementors
might use this idiom and be in for a surprise.


> Gunnar cannot figure out what I am even saying and to him equals() should
> behave naturally period and the hell with consistency.
>
> I see four ways out:
>
> 1. we stay how it is today with the weird equals behavior in case of
> constants and keep the concept that an Option is not identified by its
> value.
> 2. we move to Gunnar's position that an Option is uniquely identified by
> its family + value for unique options and keep generic options identity by
> family + key (and not value)
> 3. we move to Gunnar's position but all the way: an Option is uniquely
> identified by its family + value both for generic and unique options
> 4. we fall back to differentiate in the type system option family, option
> key, and option value.
>

My preference would be 2. The problem with 3. is that it e.g. for
NamedQueryOption would allow to have two queries with the same name but
different HQL.

I don't think there really is an inconsistency with 2. Actually one might
consider a unique option type such as ShowSql just as a special kind of
generic option family with exactly two defined options which don't have any
value (besides the key itself). So getOptionIdentifier() is based on the
key in all cases.

> - unique options like show_sql where the option family and the option are
one and the same thing

I think this is really where the difference lies. To me this option family
would actually have exactly two defined options, with the keys (identities)
TRUE and FALSE. Sticking to this model, the current implementation of
getOptionidentifier() is inconsistent to me as it once takes into account
family and key (generic options) and once only family (unique options).




>
> So what do you guys all think of this problem?
>
> Emmanuel
> _______________________________________________
> hibernate-dev mailing list
> hibernate-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/hibernate-dev
>