{"id":62,"date":"2024-02-04T18:00:00","date_gmt":"2024-02-04T18:00:00","guid":{"rendered":"https:\/\/permutationcity.co.uk\/bp\/?p=62"},"modified":"2024-04-24T18:14:43","modified_gmt":"2024-04-24T18:14:43","slug":"a-step-too-far","status":"publish","type":"post","link":"https:\/\/permutationcity.co.uk\/bp\/2024\/02\/04\/a-step-too-far\/","title":{"rendered":"A step too far"},"content":{"rendered":"\n

One of the seeds for this blog was some code that I was reviewing. It was based on a recommendation by\nSonarQube. I don’t have it to hand but it went roughly like this:<\/p>\n\n\n\n

    \/\/ Prepare query...\n    if (auto entry = FetchEntry(query); entry.Active()) {\n       \/\/ Use entry...\n    }<\/pre><\/div>\n\n\n\n
and was based on the rule:<\/p>\n\n\n\n
\n
“if”, “switch”, and range-based for loop initializer should be used to reduce scope of variables<\/p>\n<\/blockquote>\n\n\n\n
The code is perfectly correct and the rule makes sense but I don’t like the result.\nIt seems just as bizarre as seeing code like this:<\/p>\n\n\n\n
    foo(); bar();<\/pre><\/div>\n\n\n\n
C++ lets you put as many statements as you like on one like but that doesn’t mean you should do it!\nSome people out there are probably nodding, some shaking their heads and others shrugging.<\/p>\n\n\n\n
I was going to talk about everybody having their own unwritten rules. Instead I can quote another\nSonarQube rule:<\/p>\n\n\n\n
\n
Statements should be on separate lines<\/p>\n<\/blockquote>\n\n\n\n
Their rule uses exactly the same foo \/ bar example that I did. It does also list some exceptional\ncircumstances but not the specific one that lead me here.<\/p>\n\n\n\n
The “why” is important<\/h2>\n\n\n\n
Lists of rules in books, online and in automated tools are useful. They have the “what” but often don’t\nhave the “why”. I think automation in particular can end up with good rules used in a bad way.<\/p>\n\n\n\n
So lets think about the “why” behind these rules.<\/p>\n\n\n\n
\n
“if”, “switch”, and range-based for loop initializer should be used to reduce scope of variables<\/p>\n<\/blockquote>\n\n\n\n
The advantages:<\/p>\n\n\n\n
    \n
  • The variable doesn’t have to be considered when considering any other code.<\/li>\n\n\n\n
  • Resources connected to the variable can be freed earlier.<\/li>\n\n\n\n
  • The identifier can be safely re-used in a different scope.<\/li>\n<\/ul>\n\n\n\n
    \n
    Statements should be on separate lines<\/p>\n<\/blockquote>\n\n\n\n
    The advantages:<\/p>\n\n\n\n
      \n
    • A single statement is easier to reason about.<\/li>\n\n\n\n
    • Unlikely to miss any statements.<\/li>\n<\/ul>\n\n\n\n
      I already recommend using smaller functions rather than larger ones so the reduction in scope has a\nsmaller impact. Being able to re-use the variable name in a different scope might be handy but not\nessential. On the other hand, having someone miss code because it’s in a strange place seems like\na terrible idea. I want my code to be as quick and easy to understand as possible.\nFor me the first rule is nice but not as fundamental as the second rule.<\/p>\n\n\n\n
      In the code review I recommend switching to this:<\/p>\n\n\n\n
          \/\/ Prepare query...\n    auto entry = FetchEntry(query);\n    if (entry.Active()) {\n       \/\/ Use entry...\n    }<\/pre><\/div>\n\n\n\n
      It takes an extra line but, to me, each line is simpler so it’s easier to understand and less likely\nto have a problem.<\/p>\n\n\n\n
      I don’t know if you agree or disagree with my preference. I do want other developers to think about\nthe “why” of rules they are using so they can make a considered choice themselves.<\/p>\n","protected":false},"excerpt":{"rendered":"
      One of the seeds for this blog was some code that I was reviewing. It was based on a recommendation by SonarQube. I don’t have it to hand but it went roughly like this: and was based on the rule: “if”, “switch”, and range-based for loop initializer should be used to reduce scope of variables […]<\/p>\n","protected":false},"author":1,"featured_media":0,"comment_status":"open","ping_status":"open","sticky":false,"template":"","format":"standard","meta":{"_import_markdown_pro_load_document_selector":0,"_import_markdown_pro_submit_text_textarea":"","footnotes":""},"categories":[1],"tags":[8],"class_list":["post-62","post","type-post","status-publish","format-standard","hentry","category-general","tag-standards"],"_links":{"self":[{"href":"https:\/\/permutationcity.co.uk\/bp\/wp-json\/wp\/v2\/posts\/62","targetHints":{"allow":["GET"]}}],"collection":[{"href":"https:\/\/permutationcity.co.uk\/bp\/wp-json\/wp\/v2\/posts"}],"about":[{"href":"https:\/\/permutationcity.co.uk\/bp\/wp-json\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"https:\/\/permutationcity.co.uk\/bp\/wp-json\/wp\/v2\/users\/1"}],"replies":[{"embeddable":true,"href":"https:\/\/permutationcity.co.uk\/bp\/wp-json\/wp\/v2\/comments?post=62"}],"version-history":[{"count":2,"href":"https:\/\/permutationcity.co.uk\/bp\/wp-json\/wp\/v2\/posts\/62\/revisions"}],"predecessor-version":[{"id":179,"href":"https:\/\/permutationcity.co.uk\/bp\/wp-json\/wp\/v2\/posts\/62\/revisions\/179"}],"wp:attachment":[{"href":"https:\/\/permutationcity.co.uk\/bp\/wp-json\/wp\/v2\/media?parent=62"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/permutationcity.co.uk\/bp\/wp-json\/wp\/v2\/categories?post=62"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/permutationcity.co.uk\/bp\/wp-json\/wp\/v2\/tags?post=62"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}