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 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

Would you like to post a relpy?


This post is a reply to:
By atul (24. November 2006, 13:38:45)
Patch — moved readInt() to BDecoder
 (3)
Dear all As per Peter's suggestion, I moved the readInt() back to BDecoder.... Let me know ... ---- cheers atul

Follow-ups:
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 (more...)