Hi Petr,
I wanted to make a detailed review of your branch before I do the merge. I have done some minor modifications which I’ve commented below. Let me know if you disagree! The branch is developers/peter/calitko-petr rev130.
either “empty” (after an object is created) or all their data are valid.
The above statement is not quite right. Exception safety actually only regards the situations where exceptions are thrown. Whether the objects are empty or valid could be considered a “semantical” extension to the exception safety concept. I’ve just rephrased the above statement a bit.
// Implicit sharing (same data)
bool loadedOk2 = loadTorrentData (torrentFileOk1, torrent2);
CPPUNIT_ASSERT (loadedOk2);
CPPUNIT_ASSERT (torrent1 == torrent2);
When torrent2 is about to be modified the shared data should be copied. It would actually be interesting to load different data in torrent2 and make sure that torrent1 != torrent2. You actually do that right after the above code snippet but then the two objects no longer share the same data. Assuming that the private data was not copied, the above check could not reveal this error. To be precise, you do check that case right after that but the two checks seem to be redundant and confused me a bit. I’ve actually split the single test case into a few smaller ones that explicitly address the individual cases.
The test suite for TorrentParser is very extensive and I totally like how you’ve split the complex parsing operation into numerous small steps (individual functions). Even though the source file is over 700 lines the code is perfectly readable!
If you have no further corrections I will merge the branch in the official Calitko branch. Please let me know!
Regards,
Peter