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 > > >
