Re: Re: SourceComment: BItem hierarchy - Removing needless code…

Hi Peter

I think depending on types is going against the OO grain.

Then how could we design BItem? Here is one idea…

1. Push the parsing to BItem concrete classes ( So only BInt knows that it is supposed to read a number squeezed in ‘i’ and ‘e’.
2. Now each concrete BItem object peeks the char… if it is ‘i’

and the class is BInt, then only it reads off the stream. Else it calls BItem::parse…

3. BItem::parse now calls the next object… This will

again take us to step 2.

4. At the end of the chain is BErrorItem … Here we break the chain. It is like the NULL pointer in a linked list.

5. So the knowledge is completely encapsulated in the BItem hierarchy…

6. Tomorrow if any new data types is added, you know where to add its processing ;-)

                     7. This is the Chain Of Responsibility pattern….
What do you think of this design?

– cheerio atul

Peter Dimov wrote:

Hi Atul, you are perfectly right about this refactoring!

If we go a step further we can get rid of type() altogether and use the RTTI instead. For example:

if (typeid (*item) != typeid (BString))
...

I guess that typeid() is just like a virtual call anyway, so it will not be less efficient. We can get rid of type() functions in a number of classes ;-)

Is there something with typeid() I’m missing? Let me know what you think?

Best regards,

Peter

atul wrote:Dear all

While reading BDecoder code, I came upon this class. I think we are storing redundant type info here….

Here is a patch that removes a lot of code

Let me know..

Btw, I generated the patch thru bzr… This is a superb tool, thanks Peter for introducing me to it… I am going to try the vimdiff plugin today some time… ;-)

—cheers atul

Would you like to post a relpy?


This post is a reply to:
Re: SourceComment: BItem hierarchy - Removing needless code…
Hi Atul, you are perfectly right about this refactoring! If we go a step further we can get rid of type() altogether and use the RTTI instead. For example: if (more...)

Follow-ups:
Re: Re: Re: SourceComment: BItem hierarchy - Removing needless code…
Hi Atul, The chain of responsibility sounds interesting! I’ll check your patch. I think we’ll anyway need to know the type because the users of BDecoder like Torrent (file) or TrackerResponse will (more...)