By Peter Dimov (28. November 2006, 14:38:53)
Re: Re: Patch — moved readInt() to BDecoder
 (1 )

Here is a modified patch of Atul’s BDecoder refactoring. I modifed BinaryReader::readString() and use it instead of the new BinaryReader::indexOf(). Additionally, BDecoder::readString() was refactored to use tryReadInt(). All tests run of course! Further comments or I can commit now?

Regards,

Peter

Peter Dimov wrote:Hey Atul, the patch looks quite neat! This is much better than the old version. I think that using BinaryReader::readString() would make the resulting code even shorter.

Btw we should be carefull when calling functions with side effects in Q_ASSERTs. For example Q_ASSERT (reader.readByte() == 'i'); will not be compiled in a release build and the ‘i’ will not be eaten.

Should we run the unit tests on release builds of the calitko library as well?

Regards,

Peter

atul wrote:Dear all

As per Peter’s suggestion, I moved the readInt() back to BDecoder….

Let me know …
—- cheers atul

Attached Files:

calitko-0.5.6-BDecoder.patch 16K

Would you like to post a relpy?


This post is a reply to:
By Peter Dimov (24. November 2006, 14:21:27)
Re: Patch — moved readInt() to BDecoder
 (2)
Hey Atul, the patch looks quite neat! This is much better than the old version. I think that using BinaryReader::readString() would make the resulting code even shorter. Btw we should be (more...)

Follow-ups:
By Peter Dimov (30. November 2006, 00:43:58)
Re: Re: Re: Patch — moved readInt() to BDecoder
 (0)
I just committed this patch in revno 81. Peter Dimov wrote:Here is a modified patch of Atul's BDecoder refactoring. I modifed BinaryReader::readString() and use it instead of the new BinaryReader::indexOf(). (more...)