{"id":1,"date":"2010-12-11T01:30:34","date_gmt":"2010-12-11T08:30:34","guid":{"rendered":"http:\/\/blog.mezeske.com\/?p=1"},"modified":"2010-12-11T04:01:53","modified_gmt":"2010-12-11T11:01:53","slug":"hello-world","status":"publish","type":"post","link":"http:\/\/blog.mezeske.com\/?p=1","title":{"rendered":"The Terror of the Long Comment"},"content":{"rendered":"

A code com\u00adment can be a won\u00adder\u00adful thing. It can offer a gem of con\u00adtext around a quirky bit of code that will make the read\u00ader\u2019s life eas\u00adi\u00ader for years to come:<\/p>\n

\/\/ Fields c and d are intentionally switched in version 3 of the<\/span>
\n\/\/ protocol; see RFC 2324 S 12.<\/span>
\nfields.push_back<\/span>(<\/span> a )<\/span>;<\/span>
\nfields.push_back<\/span>(<\/span> b )<\/span>;<\/span>
\nfields.push_back<\/span>(<\/span> d )<\/span>;<\/span>
\nfields.push_back<\/span>(<\/span> c )<\/span>;<\/span><\/div><\/div>\n

If I were read\u00ading the above code snip\u00adpet with\u00adout the com\u00adment, I\u2019d prob\u00ada\u00adbly be very skep\u00adti\u00adcal about the order in which things were pushed into the fields con\u00adtain\u00ader. The com\u00adment cures this skep\u00adti\u00adcism and jus\u00adti\u00adfies the odd-look\u00ading but entire\u00adly cor\u00adrect code.<\/p>\n

I find that most use\u00adful com\u00adments are about the length of the one above, and they usu\u00adal\u00adly have the same pur\u00adpose: to explain some\u00adthing that would oth\u00ader\u00adwise give the read\u00ader some pause. The oth\u00ader nec\u00ades\u00adsary con\u00addi\u00adtion for a use\u00adful com\u00adment is that there isn\u2019t any obvi\u00adous way to rewrite the code so that no com\u00adment is required for it to be eas\u00adi\u00adly under\u00adstood. In the case of the above code snip\u00adpet, it could per\u00adhaps be rewrit\u00adten as follows:<\/p>\n

void<\/span> reorder_fields_for_version_3(<\/span> container&<\/span> fields )<\/span>
\n{<\/span>
\n    assert<\/span>(<\/span> fields.size<\/span>(<\/span>)<\/span> >=<\/span> 4<\/span> )<\/span>;<\/span>
\n    std::<\/span>swap<\/span>(<\/span> fields[<\/span>2<\/span>]<\/span>, fields[<\/span>3<\/span>]<\/span> )<\/span>;<\/span> \/\/ See RFC 2324 S 12.<\/span>
\n}<\/span>
\n
\nfields.push_back<\/span>(<\/span> a )<\/span>;<\/span>
\nfields.push_back<\/span>(<\/span> b )<\/span>;<\/span>
\nfields.push_back<\/span>(<\/span> c )<\/span>;<\/span>
\nfields.push_back<\/span>(<\/span> d )<\/span>;<\/span>
\nreorder_fields_for_version_3(<\/span> fields )<\/span>;<\/span><\/div><\/div>\n

I think that each of the above code snip\u00adpets could be appro\u00adpri\u00adate, depend\u00ading on its con\u00adtext. If the field-switch\u00ading only occurs in one place, the first approach would be the sim\u00adplest. Otherwise, if the field-switch\u00ading hap\u00adpens in sev\u00ader\u00adal places, the sec\u00adond approach would prob\u00ada\u00adbly be best. At any rate, if I ran across either of the solu\u00adtions in some code I was read\u00ading, I would under\u00adstand what was going on.<\/p>\n

This brings me to the top\u00adic of this post, which is that while com\u00adments can be extreme\u00adly use\u00adful, they can also be one of the most ter\u00adri\u00adfy\u00ading imple\u00adments of the con\u00adfused pro\u00adgram\u00admer \u2014 strik\u00ading pain and suf\u00adfer\u00ading direct\u00adly into the soul of the read\u00ader. Comments of this type can twist the read\u00ader\u2019s con\u00adtent\u00aded mood into one of abject pessimism:<\/p>\n

\/\/ When using this function, ensure that you obtain the<\/span>
\n\/\/ base_class_lock_ before you obtain the send_queue_lock_,<\/span>
\n\/\/ otherwise when the base class' munge() function is called,<\/span>
\n\/\/ it will update the target map prematurely -- and if that<\/span>
\n\/\/ happens, the next asynchronous call to pop_queue() will skip<\/span>
\n\/\/ over some of the records in the queue.  Normally this would be<\/span>
\n\/\/ acceptable, but other_class_x depends on the records coming<\/span>
\n\/\/ out of the queue in the order they were put in (unless<\/span>
\n\/\/ WEIRD_MACRO_Z is defined).<\/span>
\nqueue_result enqueue(<\/span> const<\/span> record&<\/span> r )<\/span>
\n{<\/span>
\n    \/* ... *\/<\/span>
\n}<\/span><\/div><\/div>\n

If you\u2019re any\u00adthing like me, you will antic\u00adi\u00adpate the pain of that com\u00adment before you start read\u00ading it, sim\u00adply because of its length. I almost nev\u00ader see six-line com\u00adments that do not hurt my brain. The only occa\u00adsion on which such a lengthy com\u00adment is real\u00adly jus\u00adti\u00adfied is when it describes some com\u00adpli\u00adcat\u00ading fac\u00adtor that is exter\u00adnal to the code itself. For exam\u00adple, a com\u00adment in a piece of client soft\u00adware might need to be lengthy to describe some bizarre behav\u00adior in the serv\u00ader it is meant to talk to. That does\u00adn\u2019t seem to be avoid\u00adable. However, if the code that one is cur\u00adrent\u00adly writ\u00ading requires a six-line com\u00adment to explain itself, that should be seen as a huge red flag. Such a long com\u00adment should tell the author to step back and look at their design, to fig\u00adure out how to sim\u00adpli\u00adfy it so that no such com\u00adments are nec\u00ades\u00adsary. For instance, the author might read that com\u00adment and ask them\u00adselves the fol\u00adlow\u00ading questions:<\/p>\n

\/\/ When using this function, ensure that you obtain the<\/span>
\n\/\/ base_class_lock_ before you obtain the send_queue_lock_,<\/span><\/div><\/div>\n

Do there need to be two locks? If so, could the send_queue_lock_ be inte\u00adgrat\u00aded direct\u00adly into the send queue? Could the lock\u00ading be fac\u00adtored out into a sep\u00ada\u00adrate func\u00adtion so that it always hap\u00adpens in the right order?<\/p>\n

\/\/ otherwise when the base class' munge() function is called,<\/span>
\n\/\/ it will update the target map prematurely -- and if that<\/span>
\n\/\/ happens, the next asynchronous call to pop_queue() will skip<\/span>
\n\/\/ over some of the records in the queue.<\/span><\/div><\/div>\n

Can the munge() func\u00adtion be changed so that it always updates the tar\u00adget map cor\u00adrect\u00adly? Maybe it should be respon\u00adsi\u00adble for obtain\u00ading its own locks. Why should pop_queue() ever be allowed to skip records? Can it be changed so that it doesn\u2019t?<\/p>\n

\/\/ Normally this would be<\/span>
\n\/\/ acceptable, but other_class_x depends on the records coming<\/span>
\n\/\/ out of the queue in the order they were put in (unless<\/span>
\n\/\/ WEIRD_MACRO_Z is defined).<\/span><\/div><\/div>\n

The user of this func\u00adtion should\u00adn\u2019t have to know about what other_class_x does. And is WEIRD_MACRO_Z<\/span> nec\u00ades\u00adsary? If it\u2019s spe\u00adcif\u00adic to other_class_x, why does the user of this func\u00adtion need to know about it?<\/p>\n

By address\u00ading a few of these ques\u00adtions, it might be pos\u00adsi\u00adble for the author to shrink that big, com\u00adpli\u00adcat\u00aded, painful, six-line com\u00adment down to a rea\u00adson\u00adable size. And by doing so, they might save some of the future main\u00adtain\u00aders of their code from devel\u00adop\u00ading severe drink\u00ading prob\u00adlems. More impor\u00adtant\u00adly, though, they will prob\u00ada\u00adbly end up with less bug\u00adgy code, because in my expe\u00adri\u00adence, the more com\u00adpli\u00adcat\u00aded rules that I have to hold in my head while writ\u00ading code, the more I screw up. The code that con\u00adtains the most lengthy, most fre\u00adquent com\u00adments is com\u00admon\u00adly the worst code.<\/p>\n","protected":false},"excerpt":{"rendered":"

A code com\u00adment can be a won\u00adder\u00adful thing. It can offer a gem of con\u00adtext around a quirky bit of code that will make the read\u00ader\u2019s life eas\u00adi\u00ader for years to come: \/\/ Fields c and d are inten\u00adtion\u00adal\u00adly switched in ver\u00adsion 3 of the \/\/ pro\u00adto\u00adcol; see RFC<\/span> 2324 S 12. fields.push_back( a ); fields.push_back( [\u2026]<\/p>\n","protected":false},"author":1,"featured_media":0,"comment_status":"open","ping_status":"open","sticky":false,"template":"","format":"standard","meta":{"wp_typography_post_enhancements_disabled":false},"categories":[3],"tags":[4,5,6],"_links":{"self":[{"href":"http:\/\/blog.mezeske.com\/index.php?rest_route=\/wp\/v2\/posts\/1"}],"collection":[{"href":"http:\/\/blog.mezeske.com\/index.php?rest_route=\/wp\/v2\/posts"}],"about":[{"href":"http:\/\/blog.mezeske.com\/index.php?rest_route=\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"http:\/\/blog.mezeske.com\/index.php?rest_route=\/wp\/v2\/users\/1"}],"replies":[{"embeddable":true,"href":"http:\/\/blog.mezeske.com\/index.php?rest_route=%2Fwp%2Fv2%2Fcomments&post=1"}],"version-history":[{"count":6,"href":"http:\/\/blog.mezeske.com\/index.php?rest_route=\/wp\/v2\/posts\/1\/revisions"}],"predecessor-version":[{"id":4,"href":"http:\/\/blog.mezeske.com\/index.php?rest_route=\/wp\/v2\/posts\/1\/revisions\/4"}],"wp:attachment":[{"href":"http:\/\/blog.mezeske.com\/index.php?rest_route=%2Fwp%2Fv2%2Fmedia&parent=1"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"http:\/\/blog.mezeske.com\/index.php?rest_route=%2Fwp%2Fv2%2Fcategories&post=1"},{"taxonomy":"post_tag","embeddable":true,"href":"http:\/\/blog.mezeske.com\/index.php?rest_route=%2Fwp%2Fv2%2Ftags&post=1"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}