Task #2494
closed
Backward-compatible refactoring of EncodingBuffer (EncodingImpl class template)
Added by Alex Afanasyev almost 10 years ago.
Updated almost 10 years ago.
Description
Right now code in encoding/encoding-buffer.hpp
is "a little bit" messy and lacks proper doxygen documentation.
This task is to do refactoring of the implementation with move of what can be moved to .cpp files.
Refactoring highlights:
namespace encoding {
class Encoder; // class (not a class template) for encoding
class Estimator; // class (not a class template) for estimation
// both Encoder and Estimator share similar interface for prepend/append
template<bool isRealEncoderNotEstimator>
class EncodingImpl;
}
typedef encoding::EncodingImpl<true> EncodingBuffer;
typedef encoding::EncodingImpl<false> EncodingEstimator;
using encoding::EncodingImpl;
Using true
false
as template parameter is ugly and confusing.
Can we declare two constants (whose values are true
and false
) and use these constants in place of true
false
everywhere?
- Status changed from Resolved to In Progress
- % Done changed from 100 to 90
We had constants defined, they were never used though...
Technically, we don't really need this bool parameter. We can changed that, but it will be a breaking change, which affects many places :(. I could be ok to start deprecation/removal cycle:
template<class T>
class EncoderEstimator;
template<>
class EncoderEstimator<Encoder> : public Encoder {
...
};
template<>
class EncoderEstimator<Estimator> : public Estimator {
...
};
(I couldn't figure out a better name for this encoder/estimator, open for suggestions. May be encoder::Impl<>
?)
- Status changed from In Progress to Code review
- % Done changed from 90 to 100
Technically, we don't really need this bool parameter. We can changed that, but it will be a breaking change, which affects many places :(. I could be ok to start deprecation/removal cycle:
Just use the constants instead of true
false
. It's not a breaking change.
- Status changed from Code review to Closed
Also available in: Atom
PDF