Path: ...!eternal-september.org!feeder3.eternal-september.org!news.eternal-september.org!eternal-september.org!.POSTED!not-for-mail From: Janis Papanagnou Newsgroups: comp.lang.c Subject: Re: Which code style do you prefer the most? Date: Wed, 26 Feb 2025 17:32:07 +0100 Organization: A noiseless patient Spider Lines: 87 Message-ID: References: <20250225104754.267@kylheku.com> <874j0g2a8u.fsf@onesoftnet.eu.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Injection-Date: Wed, 26 Feb 2025 17:32:09 +0100 (CET) Injection-Info: dont-email.me; posting-host="4f0782fc39d9cc4b721840988cfbfa82"; logging-data="2783961"; mail-complaints-to="abuse@eternal-september.org"; posting-account="U2FsdGVkX1/jmvbEiLKHG77O2TVHLVGR" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 Cancel-Lock: sha1:iT8ynoDQyKcDfr/oBdipFyuPaNo= In-Reply-To: X-Enigmail-Draft-Status: N1110 Bytes: 4796 On 26.02.2025 15:58, David Brown wrote: > On 26/02/2025 14:06, Janis Papanagnou wrote: >> On 26.02.2025 12:53, Ar Rakin wrote: >>> Janis Papanagnou writes: >>>> >>>> Re "goofy style"; I've seen someone preferring >>>> >>>> while (q(a,b)) >>>> { >>>> a=b; >>>> f(x); >>>> if (c>d) >>>> { >>>> g(y); >>>> } >>>> } >>>> >>>> To each his own. >>> >>> That looks like a nightmare for the code reviewers. > > I find that style grating. But I don't think I'd call it a "nightmare" > - I've seen far worse. > > But from all my education and training in coding, mathematics, > documentation, writing, and typography, I am a solid believer in one > rule - the most important feature of any written information is the > spacing. If I were tasked with making that code clear, the first step > would be to add a few spaces - "a = b;", "if (c > d)" - that would be > higher priority than using a more "normal" brace style such as the "One > True Brace Style". Just note that this was _ad hoc_ code only to illustrate the braces placement, not the spacing. (The guy I was talking about writes in fact readable(!) code, i.e. with spacing, sensible naming, comments, etc. etc.) The only really strange thing in his code are the braces placement that I've not seen anywhere before. (And I've seen a lot code, most of it complying more or less to prevalent conventions.) > >> >> I cannot tell where that comes from; the person who uses it is an >> experienced Perl programmer - may that be some convention in that >> specific language context? (I can't tell.) >> >> WRT code reviews; in the past I had horrendous code that I needed >> to reformat using a code-beautifier before I could review it. But >> those were extreme (and only rare) cases. (And above yet isn't as >> bad as those other artworks I had.) >> > > I think it is more common in such cases to reject the code from the > review, and insist that the author re-format it appropriately. The > reviewer's job is to /review/ the code, not fix it or clean it up. Yes, but there are situations when you have to review code where the author isn't available any more, as in the as said rare cases I reported about. In another somewhat different case we had to rewrite a piece of code to add new functionality but keep (unspecified!) existing functionality. That was not exactly a "review" but actually an analysis, close to a black-box analysis. Reformatting was the least effort compared to the behavioral analysis and stepwise refactoring, using Extreme Programming methods, Pair Programming and ad hoc introduction of regression tests. *That* was a nightmare! Yes, the simple case is to just reject code. Or to let automated tools run across the code to reject it in the first place without any human interaction. > > One thing that is important for code reviewers, and any kind of > comparisons between versions (such as for source code management > systems), is to try to reduce the number of lines that are changed > unnecessarily, and to try to make those changed lines clear. What are you trying to suggest here? (It looks like I've never had a case where it was important to compare source code with previous versions just to remove "unnecessary" lines. - This sounds really strange to me, but you may want to explain it.) Janis > [...]