views:

199

answers:

10

I have a data structure that I want to access / modify in different ways in different situations. I came up with this:

class DataStructure
{
    public:
       int getType();

    private:
       // underlying data containers    
};

class WrapperBase
{
    public:
        void wrap(DataStructure *input)
            {dataStructure = input;}

    protected:
        DataStructure *dataStructure;
};

class WrapperOne : public WrapperBase
{
     public:
          // @Mykola: I know bytes 4-10 in an array of type one specify the date
          // so this method will format those bytes and return them
          Data getDate()
};

class WrapperTwo : public WrapperBase
{
     public:
          // @Mykola: There is mostly just a chunk of data in packet type two. So this
          // method will turn 4 bytes into an int at position index and return that
          int dataAt(int index);              
};

One problem I see here is that WrapperBase isn't abstract even though it should be. I could of course add some pure virtual dummy function or an assert(0) in the constructor but that seems too hackish a solution. Or should I get rid of the inheritance entirely since it's only really done for code-reuse? Any other problems with this solution?

Or am I on the wrong track entirely?

Edit @ Paul

Why do I want to do this? Well, I get several 1000 arrays of serialized data, which I want to add to a dataset. The first few bytes of each array tell me what sort of data it is, which dictates how I have process it. So then I do something like:

// some place in the program
dataSet.processData(dataStructure);

// in the data set class
DataSet::processData(DataStructure *dataStructure)
{
     if(dataStructure->getType() == TYPE_ONE)
     {
          WrapperOne wrapperOne;
          wrapperOne.wrap(dataStructure); 
          processDataTypeOne(wrapperOne); 
     }

     // repeat the above for other data types
}

I could of course put all the logic in the processDataTypeOne function, and that was what I was doing in the beginning, but operating on the raw data structure turned into an ugly mess of index operations. That's why I'd like to wrap it in an object, which will hide all that.

+1  A: 

Well, it seems ok to me. I'd suggest writing a more thorough interface (with virtual methods) for WrapperBase, if that is possible. I am not asking you to add unnecessary functions, instead I suggest making the 'Wrapper' interface more explicit. As opposed to what Thomi has suggested, my suggestion involves identifying the interface for modification up front-- as I already said, that may not be possible.

batbrat
+1  A: 

Just one quick comment, you cant really add an assert(0) in the Base constructor since the constructor will always be run regardless of how much inheritance you have.

mizipzor
A: 

I think we need to know more about why you need to access/modify it in different ways, and why you see the need for different classes to do this, and what you expect to gain from this

Paul
A: 

I see nothing wrong with this solution.

You may well find that down the track you end up adding some common code to the WrapperBase class, and when that haoppens you'll be glad that you have a common base class.

You may also find that WrapperOne and WrapperTwo support a similar interface. If that is the case, you may wish to define that interface in WrapperBase using pure virtual methods - that way you can use a pointer-to-WrapperBase as a stand-in for whatever Wrapper you are actually using.

... hope that makes sense!

Thomi
"You may also find that WrapperOne and WrapperTwo support a similar interface" - probably not, though, since the reason for doing this at all seems to be that the access and modification is done in different ways?
Paul
+1  A: 

I think the fact that you lack any virtual methods in the base class is an indicator that inheritance is a waste of time. All WrapperOne and WrapperTwo share is that they use the same data structure. If you're trying to avoid writing the same business logic twice to interact with DataStructure, then consider wrapping DataStructure in a class to implement that business logic and have WrapperOne and WrapperTwo both use (not inherit from) that business logic class.

Pesto
+1  A: 

You could make the default and copy constructors of WrapperBase 'protected'. It doesn't require adding any non-functional methods and ensures that no classes outside the inheritance chain can invoke the constructor of WrapperBase. Or just scrap the inheritance all together.

+1  A: 

Make your wrappers to be a data.
Create factory that will return either data or different wrappers. Here is what I mean.

class DataStructure
{
public:
    typedef int DataType;

    DataStructure( int id ):
        id_( id )
    {}

    DataStructure( const DataStructure& dataStructure );
    virtual ~DataStructure();

    virtual void Set( const DataType& data ) { data_ = data; } 
    virtual DataType Get() const { return data_; }

    int id() const { return id_; }
private:
    DataType data_;
    int id_;
};

class WrapperBase : public DataStructure
{ 
public:
    WrapperBase( DataStructure* dataStructure ):
        DataStructure( dataStructure->id() ),
        dataStructure_( dataStructure )
    {}

    virtual void Set( const DataType& data );
    virtual DataType Get() const;
protected:
    DataStructure* dataStructure_;
};

class WrapperOne : public WrapperBase
{ 
public:
    WrapperOne( DataStructure* dataStructure );
    virtual void Set( const DataType& data );
    virtual DataType Get() const;
};

class WrapperTwo : public WrapperBase
{ 
public:
    WrapperTwo( DataStructure* dataStructure );
    virtual void Set( const DataType& data );
    virtual DataType Get() const;
};

DataStructure* getWrapper( DataStructure* dataStructure )
{
    switch ( dataStructure->id() )
    {
        case 1: return new WrapperOne( dataStructure );
        case 2: return new WrapperTwo( dataStructure );
        default: return new DataStructure( *dataStructure );
    }
}

void processData(DataStructure *dataStructure)
{
    std::auto_ptr<DataStructure> wrapper( getWrapper( dataStructure ) );
    processDataImpl( wrapper.get() );
}
Mykola Golubyev
You're deriving from DataStructure and at the same time keeping a pointer to a DataStructure object in your wrappers. What's the point of doing that?
drby
Derive - to be able to pass wrapper instead of DataStructure.Keep pointer - to be able to implement get and set. In the trvial case it will be dataStructure_->Get(). It could be dataStructure_->Get() * 2;
Mykola Golubyev
A: 

I'd use a teplated wrapper, that way you get what you need and don't pay anything for virtual calls/interface.

   template<T>
   class Wrapper
   { 
     Wrapper( const T& data )

You can then specialize the Wrappers with no need for a base class.

Robert Gould
Won't a templated wrapper only allow modification of the type operated upon and not the functionality?
batbrat
*data type - in this case, the term data structure is more appropriate.
batbrat
You can specialize templates and methods depending on the teplated type. It's a powerful concept.
Robert Gould
+2  A: 

Think about what you want your base class for your data to do. If you are going to save your data or display it to the screen you may want a base class with functions like ToString and ToXML.

Let the code evolve. Write out the different DataStructure classes you need. Then find the commonality and move that into the base class.

banno
A: 

On general, while I agree that the base class probably should be thought about twice and contain virtual methods, from a OO perspective there is no need at all to have the base class virtual.
If a class has an "is a" relationship to another class, it should inherit from this class. (E.g. a White shark "is a" shark which "is a" fish, but you may very well have a system full of only plain sharks or fishes.)
(As opposed to a "has a" relationship, where the other class should be a member. E.g. a car "has a" windshield.)

E Dominique