Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adverse Effects of different behavious of @Value.Default in @Value.Modifiable and @Value.Immutable #1148

Open
SpyrosKou opened this issue Feb 5, 2020 · 1 comment

Comments

@SpyrosKou
Copy link

SpyrosKou commented Feb 5, 2020

@Value.Modifiable
@Value.Immutable
interface ChangingIdType
{
 @Value.Default
 public String getId()
  {
    return UUID.randomUUID().toString();
  }
}

The semantics of @Value.Default in @Value.Modifiable and @Value.Immutable have an adverse effect in cases where values generated by @Value.Default methods are not the same. For instance automatically generated Ids.
In the case of the immutable object: If the ID is not set during construction by the builder the value will be cached and this test will always pass:

final ChangingIdType immutableChangingIdTypeSet=ImmutableChangingIdType.builder().setId("KnownID").build();
//This will pass
Assert.assertEquals(immutableChangingIdTypeSet.getId() , immutableChangingIdTypeSet.getId());

final ChangingIdType immutableChangingIdType=ImmutableChangingIdType.builder().build();
//This will also pass with an autogenerated id
Assert.assertEquals(immutableChangingIdType.getId() , immutableChangingIdType.getId());

In the case of the Modifiable object: If the ID is not set then the value returned by the method will not be cached and this test will fail:

final ChangingIdType modifiableChangingIdTypeSet=ModifiableChangingIdType.create().setId("KnownID");
//this test will pass:
Assert.assertEquals(modifiableChangingIdTypeSet.getId() , modifiableChangingIdTypeSet.getId())


final ChangingIdType modifiableChangingIdType=ModifiableChangingIdType.create();
//this test will fail, a new id will be generated each time
Assert.assertEquals(modifiableChangingIdType.getId() , modifiableChangingIdType.getId())

My proposal would be that it should be possible to have the result of the default method cached. The caching in Modifiables should be controlled by an annotation or an annotation parameter in order to maintain backwards compatibility.

In fact the current approach is consistent, but the Immutable object will call and set the @Value.Default during initialization, while the modifiable object will not. Thus any method marked with @Value.Default will be called until the setter is called.

Some non elegant work-arounds I have found to this is to set the value in the default method if a setter exists or manually setting the @Value.Default properties when using a Modifiable.

It would be nice to have an elegant solution to this.

@SpyrosKou SpyrosKou changed the title Effects of different behavious of @Value.Default in @Value.Modifiable and @Value.Immutable Adverse Effects of different behavious of @Value.Default in @Value.Modifiable and @Value.Immutable Feb 5, 2020
@SpyrosKou
Copy link
Author

The workaround I use currently is the following.

 @Value.Default
 public String getId()
  {
    final String id=UUID.randomUUID().toString();
   try {

            final Method setIdMethod = this.getClass().getMethod("setId", String.class);
            if (setIdMethod != null) {
                setIdMethod.invoke(this, id);
            }
        } catch (final Exception e) {
            //Ignore on purpose.
        }
  }
  

This workaround tries to call the setId(String) method if available by using reflection.
This way the getId() will always return the same value when called.
If this approach is not used the getId() will always return a new id as each call will result to an invocation of the UUID.randomUUID().toString();

Using @Value.Lazy or @Value.Derived is not an option as it does not allow setting the value, which is needed e.g. in seriliazation.
The downside is apart from the non optimal performance of reflection and try/catch that this has to be manually maintained.
For instance the name of the "getId" should be maintained in sync with any potential Style updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant