Task #2494
closed
Backward-compatible refactoring of EncodingBuffer (EncodingImpl class template)
Added by Alex Afanasyev about 9 years ago.
Updated about 9 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
Updated by 
Updated by