[hibernate-dev] Static constants in org.hibernate.engine.internal.Versioning

[Hardy Ferentschik]

Hi,

I am also for moving to enums. I am not sure whether or why this should go  
to
org.hibernate.engine.spi though. I am still at odds with this package  
refactoring.

I think we discussed once before that there are several places in the code  
which
would benefit from the introduction of an enum which can be used by  
annotations and
hbm. I think we will find more than just OptimisticLockMode.

Regarding CascadeStyle I definitely had a consolidation in mind as well.  
Really
org.hibernate.engine.spi.CascadeStyle has to become a enum. The reason I  
did not
do it was that this class is used outside the new metamodel code  
(obviously) and
I tried so far to stay away from changes which affect existing/running  
code.
Now that the integration work into the Persisters and so on has started it  
might be
the right time to start doing this type of things as well.

Another thought regarding this class is that it currently contains some  
logic. I am
not a big fan of having a logic in an emum. CascadeStyle might be  
borderline,
but it is something to keep in mind.

--Hardy


On Fri, 24 Jun 2011 02:43:16 +0200, Steve Ebersole <steve at hibernate.org>  
wrote:

> On 06/23/2011 05:42 PM, Gail Badner wrote:
>> I just ran into an issue where EntityBinding.getOptimisticLockMode()  
>> returns an ordinal from a org.hibernate.annotations.OptimisticLockType  
>> enum that is inconsistent with the static constants in  
>> org.hibernate.engine.internal.Versioning.
>>
>> Jira issue:  
>> http://opensource.atlassian.com/projects/hibernate/browse/HHH-6362
>>
>> I'm not clear on what conventions there are for enums that are  
>> source-specific.
>>
>> My preference would be to:
>>
>> 1) Extract the static constants for version type from  
>> org.hibernate.engine.internal.Versioning into a new enum that is an  
>> SPI, org.hibernate.engine.spi.OptimisticLockMode.
>>
>> 2) Change EntityBinding.getOptimisticLockMode() and  
>> EntityBindingState.getOptimisticLockMode() to return  
>> org.hibernate.engine.spi.OptimisticLockMode.
>>
>> 3) Change sources to be responsible for converting from a  
>> source-specific value to org.hibernate.engine.spi.OptimisticLockMode.
>
> Aside from the enum name, I think this is all goodness.  These are
> distinctly different from LockModes and we should avoid that confusion.
>   I tend to like to use the term "strategy" with pluggable interfaces,
> but I think OptimisticLockingStrategy is definitely descriptive here.
> Anyone else with better names?
>
>
>> In the same vein, I see there is  
>> org.hibernate.metamodel.binding.CascadeType, which is not  
>> source-specific. In that class, there's a TODO to integrate it with  
>> org.hibernate.engine.spi.CascadeStyle.
>>
>> I also see that AttributeBindingState.getCascadeTypes() returns  
>> Set<CascadeType>.
>>
>> My preference would be to remove  
>> org.hibernate.metamodel.binding.CascadeType and to change  
>> AttributeBindingState.getCascadeTypes() and  
>> AbstractAttributeBinding.getCascadeTypes() to:
>>
>> org.hibernate.engine.spi.CascadeStyle getCascadeStyle();
>
> I think this is fine.  The comment is Hardy's though, so lets let him
> respond to see if he had something else in mind.