Please coment solution (Was: Design problem...)

S

SpOiLeR

I have 4 different text file formats which basically
contain same data differently formated. I have class ParsedData which
handles data from this files. I also have 4 parses which can read from
files into ParsedData. Parsers should never be called explicitly (they
should only be called from ParsedData ctor). What is the best way to
protect those parsers?

Well, If someone is still interested, this is what I did. Coments are
welcome...

Before, I've had something like this:

class ParsedData
{
public:
ParsedData (const FileInfo& f_in);
// FileInfo is declared elsewhere and contains some data about
// file to be parsed...
~ParsedData ();

/* Set/Acess interface to data members */

private:
/* Few data members */
FileInfo fi;

void parser1 ();
void parser_helper11 ();
void parser_helper12 ();
void parser_helper13 ();
// ... Another 3 parsers similary declared
};

ParsedData::parsedData (const FileInfo& fi_in) : fi(fi_in)
{
if (condition1) parser1();
else if (condition2) parser2();
else if (condition3) parser3();
else parser4 ();
}

Now I have this:

class ParsedData {
public:
ParsedData (const FileInfo& f_in);
~ParsedData

/* Set/Acess interface to data members */
private:
/* Few data members */
FileInfo fi;
};

ParsedData::parsedData (const FileInfo& fi_in) : fi(fi_in), ...
// Initialization of all ParsedData members
{
if (cond1) parser1 par(this);
else if (cond2) parser2 par(this);
else if (cond3) parser3 par(this);
else parser4 par(this);

par.read();
}

class BasicParser // Comon interace to all parsers
{
public:
BasicParser (ParsedData *rv) : p(rv) {}
virtual ~BasicParser ();

int read ();
int write ();

private:
ParsedData *p;
};

clas ParserOne : public BasicParser
{
...

Each ParserX recieves pointer to ParsedData object and imediatelly parses
file into it. Info about path/file is included in ParsedData object...
Also, all parsers are in subnamespace of ParsedData namespace. I achieved
clearing up of ParsedData interface which was primarily my goal.
 
V

Victor Bazarov

SpOiLeR said:
Well, If someone is still interested, this is what I did. Coments are
welcome...

I don't want to spoil your triumph, yet a reply is in order.
Before, I've had something like this:

[...]
Now I have this:

class ParsedData {
public:
ParsedData (const FileInfo& f_in);
~ParsedData

/* Set/Acess interface to data members */
private:
/* Few data members */
FileInfo fi;
};

ParsedData::parsedData (const FileInfo& fi_in) : fi(fi_in), ...
// Initialization of all ParsedData members
{
if (cond1) parser1 par(this);
else if (cond2) parser2 par(this);
else if (cond3) parser3 par(this);
else parser4 par(this);

par.read();

Now, this is not C++. At least not the C++ I know. If you meant to
initialise 'par' differently based on conditions, you still have to
declare it _before_ the first 'if' so that it would be around after
the last 'else'.
}

class BasicParser // Comon interace to all parsers
{
public:
BasicParser (ParsedData *rv) : p(rv) {}
virtual ~BasicParser ();

int read ();
int write ();

Is there a reason why those are not virtual? If they are supposed to
be virtual, shouldn't they also be pure?
private:
ParsedData *p;
};

clas ParserOne : public BasicParser
{
...

Each ParserX recieves pointer to ParsedData object and imediatelly parses
file into it. Info about path/file is included in ParsedData object...

And so every parser (either by itself or through the BasicParser) has to
know how to get to the inner ParsedData contents. So, to achieve that you
need to either have them all declared "friends" (which was what you tried
to avoid in the first place), or have extensive interface that ParsedData
publishes, and _everybody_ is free to use [gasp!]...
Also, all parsers are in subnamespace of ParsedData namespace. I achieved
clearing up of ParsedData interface which was primarily my goal.

Sounds good. The idea is that you should end up with a solution that
works [for you]. I don't see a working solution as of yet, but I will
gladly take your word for it.

V
 
S

SpOiLeR

I don't want to spoil your triumph, yet a reply is in order.

What triumph?
Now, this is not C++. At least not the C++ I know. If you meant to
initialise 'par' differently based on conditions, you still have to
declare it _before_ the first 'if' so that it would be around after
the last 'else'.
True, I was giving simplified example, not the real code.
Is there a reason why those are not virtual? If they are supposed to
be virtual, shouldn't they also be pure?

The are both pure virtual (in fact there are four methods in interface of
BasicParser and all four are pure virtuals). Again, this is not real code
pasted form my IDE. I'm sorry for confusion.
private:
ParsedData *p;
};

clas ParserOne : public BasicParser
{
...

Each ParserX recieves pointer to ParsedData object and imediatelly parses
file into it. Info about path/file is included in ParsedData object...

And so every parser (either by itself or through the BasicParser) has to
know how to get to the inner ParsedData contents. So, to achieve that you
need to either have them all declared "friends" (which was what you tried
to avoid in the first place), or have extensive interface that ParsedData
publishes, and _everybody_ is free to use [gasp!]...

I did the friends part. This seemed safer and cleaner than big, messy
interface to ParsedData. Any ideas of problems that can arise? I see none,
that's why I asked for comments.
I don't see a working solution as of yet, but I will
gladly take your word for it.

Are you beeing ironic?
 
V

Victor Bazarov

SpOiLeR said:
]
I don't see a working solution as of yet, but I will
gladly take your word for it.

Are you beeing ironic?

A little. You wrote that you had a solution and you didn't
even post "real code". That's why I said that I'd take your
word for it. Whatever you posted didn't look like anything
qualifying as "a solution".

Never mind. Let's move on, shall we?
 
S

SpOiLeR

SpOiLeR said:
]
I don't see a working solution as of yet, but I will
gladly take your word for it.

Are you beeing ironic?

A little. You wrote that you had a solution and you didn't
even post "real code".

Thought it would be easier to read if I post only important parts. Once
more, sorry for confusion.
That's why I said that I'd take your
word for it. Whatever you posted didn't look like anything
qualifying as "a solution".

Never mind. Let's move on, shall we?

Sure... Thanks for your time...
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,744
Messages
2,569,482
Members
44,901
Latest member
Noble71S45

Latest Threads

Top