tags:

views:

82

answers:

2

Continuing from the question that I asked here: C++ multi-dimensional data handling

In my example: I have many Chips, each Chip has many Registers, each Register has many Cells, and each Cell has many Transistors. I asked whether to use one complex STL container for them, or to implement full classes for them. And, as advised, I chose to implement full classes for them. I have:

class Chip
{
    map<RegisterLocation, Register> RegistersPerLocation;
};

class Register
{
    map<CellLocation, Cell> CellsPerLocation;
};

// etc..

Now, I need to fill the data to the classes, and I can't decide: Should reading the data be responsibility of these classes, or should they just wrap the containers and the reading will be done outside.

I mean I have to choose one of the following: Either:

class Chip
{
    map<RegisterLocation, Register> RegistersPerLocation;
  public:
    void AddRegisterPerLocation(RegisterLocation, Register); 
};
void ReadChipData(Chip & chip)
{
    for (RegisterLocation loc = 0; loc < 10; loc++)
    {
        Register reg;
        ReadReg(reg);
        chip.AddRegisterPerLocation(loc, reg);   
    }
}     
void ReadReg(Register & reg)
{
    for (CellLocation loc = 0; loc < 10; loc++)
    {
        Cell cell;
        ReadCell(cell);
        reg.AddRegisterPerLocation(loc, cell);   
    }
}
//etc...

Or:

class Chip
{
    map<RegisterLocation, Register> RegistersPerLocation;
  public:
    void ReadData(); 
};
void Chip::ReadData() 
{
    for (RegisterLocation loc = 0; loc < 10; loc++)
    {          
        Register reg;
        reg.ReadData();
        RegistersPerLocation[loc] = reg;   
    }
}
//etc...
void ReadChipData(Chip & chip)
{
    chip.ReadData();
}

Thank you.

A: 

Making classes responsible for their serialization is better - once you change the class fields you have to change the same class serializeation method, not some reader/writer code over there.

sharptooth
I strongly disagree. This is how you build classes that can never be reused. Ever. Separate data from action, that is the correct way.
Benoît
Classes are exactly for binding action to data. Encapsulation is believed to be good.
sharptooth
@sharptooth: You are correct to a certain extent. You bind not just any action, but a valid behavior. Another thing to keep in mind is separation of concerns. Serializability is not a `Chip`'s intrinsic behavior -- modeling that into the domain object would be unfair IMO. YMMV :)
dirkgently
@dirkgently: agreed :)
Benoît
I'd say "represent the concept of a chip" is a very different responsibility than "serialize a chip class". Different tasks should be in different classes. If you bundle everything together just because it "operates on the same class", you end up with a few god classes doing everything.
jalf
Well, I agree that god classes are bad, but why can't a simple container (a set of data members and a set of getters/setters) also serialize itself?
sharptooth
+2  A: 

If you are thinking of tying the reader/writer to the domain objects in order to follow the principle of encapsulation, you are correct to a certain extent. But remember: You bind not just any action, but a valid behavior. Valid as in makes sense for the object in the domain.

Another thing to keep in mind is separation of concerns. Serializability is not a Chip's intrinsic behavior -- modeling that into the domain object would be unfair IMO. YMMV.

Separate the reading(and writing) from the classes. As the library does. Expose iterators if you have to. And you can overload the '<<' and '>>' operators for syntactic sugar ;)

A minor nit on the classes -- a template based approach looks so promising.

Here's some code I cooked up: you can try the following as well. (I've successfully compiled and run this on a MS VS2005 but check it out on your system. Also, can someone fix the tabbing -- feeling too lazy to do this :P)

/*+-----------8<----------------------------8<-----------+*/
#include <vector>
#include <iostream>
#include <algorithm>
#include <map>
#include <iterator>

/* mother template */
template<class _Item>
struct Hardware
{       
    Hardware() : _myCont(2 + ::rand() % 5) {}
private:
    typename vector<_Item> _myCont;

    // i/o friends
    template<class _Item> 
    friend ostream& operator<<(ostream& output, 
                               const Hardware<_Item>& me);
    template<class _Item>
    friend istream& operator>>(istream& in, 
                               const Hardware<_Item>& me);

};

/* actual domain objects */
/* base object */
struct Transistor {
};
/* built objects */
typedef Hardware<Transistor> Cell;
typedef Hardware<Cell> Register;
typedef Hardware<Register> Chip;

/* poorman's introspection utility */
template<class T>
const char *who() { return ""; }

template<>
const char *who<Transistor>() { return "Transistor"; }

template<>
const char *who<Cell>() { return "Cell"; }

template<>
const char *who<Register>() { return "Register"; }

template<>
const char *who<Chip>() { return "Chip"; }

/* writer/serialize out */
template<class T>
ostream& operator<<(ostream& out, const Hardware<T>& hw) {
    // whatever you need to do to write
    // os << chip works fine, because you will provide a specialization
    out << "[ " << ::who<Hardware<T>>() << " ]\n\t";

    std::copy(hw._myCont.begin(), hw._myCont.end(), 
        std::ostream_iterator< T >(std::cout, "\n\t"));

    return out;
}

/* specialize for base object */
ostream& operator<< (ostream& out, const Transistor& hw) {
    out << "[ " << ::who<Transistor>() << " ]\n";
    return out;
}


/* reader/serialize in */
template<class T>
istream& operator>>(istream& in, const Hardware<T>& hw) {
    // whatever you need to do to read
    // similarly in >> chip works fine, 
    return in;
}

// driver showing relationships
// Chip -> Register -> Cell -> Transistor
int main() {
    Transistor t;
    std::cout << t << std::endl;
    Cell ce;
    std::cout << ce << std::endl;
    Register r;
    std::cout << r << std::endl;
    Chip C;
    std::cout << C << std::endl;
}
/*+-----------8<----------------------------8<-----------+*/

Caveat: Haven't tested, so there may be quite a few compiler errors/warnings. But this should give you an idea of what I am trying to say.

dirkgently