accumulo-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Christopher Tubbs (JIRA)" <>
Subject [jira] [Commented] (ACCUMULO-1972) Range constructors call overridable method
Date Thu, 09 Nov 2017 00:26:00 GMT


Christopher Tubbs commented on ACCUMULO-1972:

Thanks for the patch, [~coffeethulhu].

I have some concerns about the patch.

Doing nothing means it is possible for careful developers to override the method. Applying
this fix prevents users from doing the thing they needed to do that would have caused the
bug in the first place (that is, override the method). I don't think this is the best fix
we can do, as it breaks the API and prevents the only use case that would have been affected
by the bug.

It seems to me that a better fix might be to move the implementation to a private method,
which can be safely called in the constructor, and also be called in the public method. That
way, if the user overrides the public method, the behavior of the constructor will be unaffected.
This fix would also allows us to avoid an API breakage, which means we can fix it in 1.7.x
and 1.8.x, instead of waiting for 2.x.

> Range constructors call overridable method
> ------------------------------------------
>                 Key: ACCUMULO-1972
>                 URL:
>             Project: Accumulo
>          Issue Type: Bug
>    Affects Versions: 1.4.4, 1.5.0
>            Reporter: Bill Havanki
>            Priority: Minor
>              Labels: newbie
>         Attachments: accumulo-1972.patch
> Several {{Range}} constructors call {{Range.beforeStartKey()}}, which is not final. This
is dangerous:
> bq. The superclass constructor runs before the subclass constructor, so the overriding
method in the subclass will get invoked before the subclass constructor has run. If the overriding
method depends on any initialization performed by the subclass constructor, the method will
not behave as expected.  ??Item 17, Effective Java Vol. 2, Bloch??
> If {{beforeStartKey()}} cannot be made final, the code should be refactored to make the
constructors safe.

This message was sent by Atlassian JIRA

View raw message