Create class ByteVecValue from typedef #2368
Conversation
tiledb/sm/misc/types.h
Outdated
|
|
||
| class ByteVecValue { | ||
| typedef std::vector<uint8_t> Base; | ||
| std::vector<uint8_t> x; |
There was a problem hiding this comment.
I think the tiledb convention is to always use underscore suffixed member variable names, i.e. 'x_'.
There was a problem hiding this comment.
Agreed this is the convention elsewhere in the code.
| explicit operator bool() const noexcept { | ||
| return !x.empty(); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Personal Opinion...
I personally find the code making use of this operator bool() less readable than the ref's to some mnemonic function (as with the prior use of ref's to '.empty()' or its negation) would be (was.)
Unless there's some well-known convention I'm simply not familiar (enough?) with (intuitiveness being really related to what one is familiar with), it's generally not intuitive to the world at large (or is it just me?) what assert()ing or if()ing a given variable may really mean, so reading code, it's necessary to know/remember what that operator bool() was actually coded to represent, i.e. what does 'true' or 'false' actually mean for some arbitrary (typed) variable.
There was a problem hiding this comment.
Conversion to bool has been embraced by C++ generally. "True" means "present" and "false" means "absent". It's a generalization of the use of a pointer as a boolean. It's now also used in the standard library for smart pointers and optional, both of which define operator bool.
More importantly, this class is transitional. None of these uses should be here at all within a few months.
…d eliminating its use an untyped variable. Added a number of methods for existing patterns of use. Forwarded a number of methods necessary for legacy behavior.
eric-hughes-tiledb
force-pushed
the
eh/create-transitional-ByteVecValue
branch
from
e2e0340
to
c2b5cd4
Compare
This PR is a transitional change toward eliminating untyped variables. It converts typedef
ByteVecValueinto a class. It has a number of methods to substitute for existing patterns of use, hiding implementation details and highlighting the results. Not all of these can be changed in this initial PR. In some cases no class exists that would adequately capture how the variable is being used. In other cases there are interactions with untyped read/write from buffers that need modifications in those classes.A number of vector methods are forwarded for legacy behavior. Not all of them could be, but notably there's no more use of
operator[]. It is not a goal of this PR to eliminate all of them; it will have to be a multi-step process.The scope of this PR is to make just the changes already here. It's intent is to be pure refactoring, with zero changes in behavior.
TYPE: IMPROVEMENT
DESC: Create class ByteVecValue from typedef