Hi Christoph, thanks for the review.
> I think it would ease understanding of the code if you would also > create a method DumpWriter::start_dump_segment(). That's a feature, so you only have to check in one place. > Also, all that DumpWriter::end_sub_record() does is asserting and > debug only. So, maybe the whole method and its calls could be > enclosed in debug_only? I would hope the compiler is smart enough to eliminate the call. > Then, I've spotted a little spelling error: line 623 - segement should be > segment. Will fix it > Then, _sub_record_ended could be changed to _in_sub_record and the > according semantics be adapted. I found that to be more understandable - > but maybe it's just a personal taste thing. I think you're right. I will rename it accordingly. > And a last point is that there are many places where sizes are calculated, > e.g. lines 1002, 1032-1036, 1081, 1116, 1174, 1175. Here, I think code > comments could be added/improved to facilitate quicker understanding > for the folks that are ingenuous to this code. I always did the size calculations in the same order the entries are written in, so it should be clear which term corresponds to which write statement. I have to try out how long the comments would get and if they facilitate a better understanding. Best regards, Ralf
