trjhtr

Please review my solution for the following problem. I am interested in:

• What do you think of the design


• Improvement tips


• Usability


• Efficiency

Problem: Need to be store policy-based objects with different template parameters in one container, and reach the common interface.

Solution:
C++ Shell

#include <exception>
#include <iostream>
#include <memory>
#include <string>
#include <vector>

class SinglePolicy

public:
 SinglePolicy() = default;

 void push(int data)
 
 std::cout << "set to SinglePolicyn";
 buffer_ = data;
 

 int pop()
 
 std::cout << "return from SinglePolicyn";
 int data = buffer_;

 buffer_ = -1;

 return data;
 

protected:
 int buffer_;
;

class VectorPolicy

public:
 VectorPolicy() = default;

 virtual void push(int data)
 
 std::cout << "emplace_back to VectorPolicyn";
 buffer_.emplace_back(data);
 

 virtual int pop()
 
 std::cout << "pop front from VectorPolicyn";
 if(buffer_.empty())
 return -1;

 int data = buffer_[0];
 buffer_.erase(buffer_.begin());

 return data;
 

protected:
 std::vector<int> buffer_;
;

template<class BufferPolicy>
class PolicyBasedBuffer
 : public BufferPolicy

public:
 PolicyBasedBuffer()
 : BufferPolicy()
 
;

typedef std::shared_ptr<PolicyBasedBuffer<SinglePolicy>> SinglePolicyPtr;
typedef std::shared_ptr<PolicyBasedBuffer<VectorPolicy>> VectorPolicyPtr;

class PolicyInterface

public:
 PolicyInterface(const std::function<void(int)>& pushArg, const std::function<int(void)>& popArg)
 : push(pushArg)
 , pop(popArg)
 

 const std::function<void(int)> push;
 const std::function<int(void)> pop;
;

typedef std::shared_ptr<PolicyInterface> PolicyInterfacePtr;

PolicyInterfacePtr saveToPolicyInterface(const SinglePolicyPtr& aPolicy)

 return std::make_shared<PolicyInterface>([aPolicy](int data) aPolicy->push(data); , [aPolicy]() return aPolicy->pop(); );


PolicyInterfacePtr saveToPolicyInterface(const VectorPolicyPtr& aPolicy)

 return std::make_shared<PolicyInterface>([aPolicy](int data) aPolicy->push(data); , [aPolicy]() return aPolicy->pop(); );


int main()

 auto singleBuffer = std::make_shared<PolicyBasedBuffer<SinglePolicy>>();
 auto vectorBuffer = std::make_shared<PolicyBasedBuffer<VectorPolicy>>();

 const int seven = 7;

 std::cout << "PolicyBasedBuffer test:n";
 singleBuffer->push(seven);
 if(seven != singleBuffer->pop())
 throw std::runtime_error("SinglePolicy :: wrong return data");

 vectorBuffer->push(seven);
 if(seven != vectorBuffer->pop())
 throw std::runtime_error("VectorPolicy :: wrong return data");

 std::vector<PolicyInterfacePtr> policy;

 policy.push_back(saveToPolicyInterface(std::make_shared<PolicyBasedBuffer<SinglePolicy>>()));
 policy.push_back(saveToPolicyInterface(std::make_shared<PolicyBasedBuffer<VectorPolicy>>()));

 std::cout << "nPolicyInterface test:n";

 for(size_t i=0; i < policy.size(); ++i)
 
 policy[i]->push(seven);
 if(seven != policy[i]->pop())
 throw std::runtime_error("policyInterface :: wrong return data");
 

 std::cout << std::flush;

Few minor notes.

1. You have deliberately assigned -1 to represent undef, correct? The question whether you'll ever need to store -1 aside, you should probably define a named constant to get rid of magic number (it also helps when you decide to accept -1s). BTW, the minimal int (-2³¹ or so) is maybe a better alternative.




2. Every constructor and factory function accepts arguments by const ref. While acceptable for shared_ptr args, it makes sense to accept others (such as closures) by value and then move them inside the object's members.




3. const_cast is a threat to PolicyInterface's public push and pop methods.




4. VectorPolicy pushes elements to its right end but pops them on the left. If it should work as a LILO queue, std::deque is a better alternative to std::vector. Otherwise, VectorPolicy should follow stack (LIFO) idiom, and elements popped at the end.

I’m having trouble following this code.

You have classes SinglePolicy and VectorPolicy that look like they should be polymorphic versions of an abstract policy because they have the same push and pop functions which are declared virtual. But they are not derived from anything.

Then, you have a template PolicyBasedBuffer which as far as I can tell doesn’t do anything at all. It derives from one of the above classes, but then adds no functionality or members at all. So why?

Then you have PolyInterface which holds pointers to push and pop functions, with free functions to populate with the push/pop from the original Policy classes.

That’s what the base class would have done automatically.

I think what you want is:

class PolicyInterface 
public:
 virtual void push(int data) = 0;
 virtual int pop() = 0;
;

class VectorPolicy : public PolicyInterface {
public:
 void push(int data) override 
 
 // ⋯ your implementation here
 
 ⋮

and likewise for the ScalarPolicy — derive it from PolicyInterface.

Now, you can have:

std::vector<unique_ptr<PolicyInterface>> policies;
policies.push_back(make_shared<VectorPolicy>());
policies.push_back(make_shared<ScalarPolicy>());
 ⋮
policies[i]->push(seven);

You mentioned policy-based objects with different template parameters in your problem statement. I don’t see any need for a template at all here, but perhaps the concrete example code simply doesn’t show the need?

The overall idea here is still the right thing. The derived class can be a template, which inherits from the interface. Stated like that, it sounds a lot like what your original code did! But you were deriving a meaningless class from the needed class, not deriving the needed class (which might happen to be templated) from an interface. Since you had virtual functions in the policy classes, I think you were confused and this is what you were trying to do.

template <typename T, size_t N>
class FlexPolicy : PolicyInterface 
 ⋮
;

policies.push_back( make_unique<FlexPolicy<string,5>> );

Misc tip: use the std::stack container adaptor instead of a plain vector.

I like your use of the C++shell web site for holding the code. Too bad it doesn’t have C++17 ☹.