What problem are you trying to solve? It seems like you're proposing refactoring for refactoring's sake.
On Mon, Aug 26, 2019, 7:13 PM Fan Liya <liya.fa...@gmail.com> wrote: > Hi Jacques, > > Thanks for the valuable feedback. > > I agree that it is a good idea to explicitly make field vectors final. > > However, I think there are still lots of duplicated code for > getInt/getLong/setInt/setLong/setIntSafe/setLongSafe. In addition, we have > a large number of time related vector classes. > > So after making field vectors final, maybe we can: > 1. Extract static utility methods for get/set int/long methods. > 2. Push the get/set int/long methods to BaseFixedWidthVector base class. > > What do you think? > > Best, > Liya Fan > > On Tue, Aug 27, 2019 at 7:16 AM Jacques Nadeau <jacq...@apache.org> wrote: > > > I think you'd have to refractor differently since one of the patterns is > > get(index, object holder) is consistently matched. That means you should > > never subclass a field vector I believe. In fact we should probably just > > formally finalize them. > > > > Once you do that, I'm not sure how much duplicated code you're actually > > removing. > > > > On Mon, Aug 26, 2019, 6:13 PM Fan Liya <liya.fa...@gmail.com> wrote: > > > > > Dear all, > > > > > > Currently, we have two classes of time related vectors. One class are > > named > > > TimeXXVector, while the other class are named TimeStampXXVector. We > found > > > there are some redundant code for these classes, so we want to do some > > > refactorings for them. > > > > > > 1. For TimeXXVector, all classes extend BaseFixedWidthVector. > Internally, > > > they are just IntVector or BigIntVector. The only differences are: > > > > > > 1) minor type > > > 2) the result of getObject method. > > > > > > Proposed way of refactoring: we make them extend IntVector or > > BigIntVector, > > > so much get/set code for int/big int can be reused. > > > > > > 2. For TimeStampXXVector, all classes extend TimeStampVector. > Internally, > > > they are BigIntVectors. > > > > > > Proposed way of refactoring: we remove the TimeStampVector, and make > all > > > classes extend BigIntVector, so much of the get/set methods can be > reused > > > from super class. > > > > > > Please give your valuable feedback. > > > > > > Best, > > > Liya Fan > > > > > >
