Project

General

Profile

Actions

Task #2494

closed

Backward-compatible refactoring of EncodingBuffer (EncodingImpl class template)

Added by Alex Afanasyev about 9 years ago. Updated about 9 years ago.

Status:
Closed
Priority:
Normal
Category:
Base
Target version:
Start date:
Due date:
% Done:

100%

Estimated time:

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;
Actions #1

Updated by Junxiao Shi about 9 years ago

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?

Actions #2

Updated by Alex Afanasyev about 9 years ago

  • Status changed from Resolved to In Progress
  • % Done changed from 100 to 90
Actions #3

Updated by Alex Afanasyev about 9 years ago

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

Actions #4

Updated by Alex Afanasyev about 9 years ago

  • Status changed from In Progress to Code review
  • % Done changed from 90 to 100
Actions #5

Updated by Junxiao Shi about 9 years ago

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.

Actions #6

Updated by Alex Afanasyev about 9 years ago

  • Status changed from Code review to Closed
Actions

Also available in: Atom PDF