Turns out writing a book and getting it published with the title "Clean Code" is great marketing.

I have had so many discussions about that style where I tried to argue it wasn't actually simpler and the other side just pointed at the book.

▲ bluecalm 3 days ago | parent | next [-]

It's like with goto. Goto is useful and readable in quite a few situations but people will write arrow like if/else tree with 8 levels of indentation just to avoid it because someone somewhere said goto is evil.

▲ zahlman 3 days ago | parent | next [-]

Funny how my Python code doesn't have those arrow issues. In C code, I understand some standard idioms, but I haven't really ever seen a goto I liked. (Those few people who are trying to outsmart the compiler would make a better impression on me by just showing the assembly.)

IMX, people mainly defend goto in C because of memory management and other forms of resource-acquisition/cleanup problems. But really it comes across to me that they just don't want to pay more function-call overhead (risk the compiler not inlining things). Otherwise you can easily have patterns like:

  int get_resources_and_do_thing() {
      RESOURCE_A* a = acquire_a();
      int result = a ? get_other_resource_and_do_thing(a) : -1;
      cleanup_a(a);
      return result;
  }

  int get_other_resource_and_do_thing(RESOURCE_A* a) {
      RESOURCE_B* b = acquire_b();
      int result = b ? do_thing_with(a, b) : -2;
      cleanup_b(b);
      return result;
  }

(I prefer for handling NULL to be the cleanup function's responsibility, as with `free()`.)

Maybe sometimes you'd inline the two acquisitions; since all the business logic is elsewhere (in `do_thing_with`), the cleanup stuff is simple enough that you don't really benefit from using `goto` to express it.

In the really interesting cases, `do_thing_with` could be a passed-in function pointer:

  int get_resources_and_do(int(*thing_to_do)(RESOURCE_A*, RESOURCE_B*)) {
      RESOURCE_A* a;
      RESOURCE_B* b;
      int result;
      a = acquire_a();
      if (!a) return -1;
      b = acquire_b();
      if (!b) { cleanup_a(a); return -2; }
      result = thing_to_do(a, b);
      cleanup_b(b); cleanup_a(a);
      return result;
  }

And then you only write that pattern once for all the functions that need the resources.

Of course, this is a contrived example, but the common uses I've seen do seem to be fairly similar. Yeah, people sometimes don't like this kind of pattern because `cleanup_a` appears twice — so don't go crazy with it. But I really think that `result = 2; goto a_cleanup;` (and introducing that label) is not better than `cleanup_a(a); return 2;`. Only at three or four steps of resource acquisition does that really save any effort, and that's a code smell anyway.

(And, of course, in C++ you get all the nice RAII idioms instead.)

▲ 8note 2 days ago | parent | next [-]

> if (!b) { cleanup_a(a); return -2; }

this rings alarm bells for me reading that a cleanup_c(c) has maybe been forgotten somewhere, since the happy and unhappy paths clean up different amounts of things.

i imagine your python code escapes the giant tree by using exceptions though? that skips it by renaming and restructuring the goto, rather than leaving out the ability to jump to a common error handling spot

	▲	zahlman 2 days ago \| parent [-]
		> this rings alarm bells for me reading that a cleanup_c(c) has maybe been forgotten somewhere, since the happy and unhappy paths clean up different amounts of things. The exact point of taking the main work to a separate function is so that you can see all the paths right there. Of course there is no `c` to worry about; the wrapper is so short that it doesn't have room for that to have happened. The Python code doesn't have to deal with stuff like this because it has higher-level constructs like context managers, and because there's garbage collection. `def get_resources_and_do(action): with get_a() as a, get_b() as b: action(a, b)`

▲ bluecalm 3 days ago | parent | prev [-]

You're assuming function calls or other constructs are more readable and better programming. I don't agree. Having a clear clean-up or common return block is a good readable pattern that puts all the logic right there in one place.

Jumping out of the loop with a goto is also more readable than what Python has to offer. Refactoring things into functions just because you need to control the flow of the program is an anti pattern. Those functions add indirection and might never be reused. Why would you do that even if it was free performance wise?

This is why new low level languages offer alternatives to goto (defer, labelled break/continue, labelled switch/case) that cover most of the use cases.

Imo it's debatable if those are better and more readable than goto. Defer might be. Labelled break probably isn't although it doesn't matter that much.

Python meanwhile offers you adding more indirection, exceptions (wtf?) or flags (inefficient unrolling and additional noise instead of just goto ITEM_FOUND or something).

▲ Scarblac 3 days ago | parent | prev | next [-]

A colleague recently added a linter rule against nested ternary statements. OK, I can see how those can be confusing, and there's probably a reason why that rule is an option.

Then replaced a pretty simple one with an anonymous immediately invoked function that contained a switch statement with a return for each case.

Um, can I have a linter rule against that?

▲

zahlman 3 days ago | parent [-]

I guess "anonymous IIFE" is the part that bothers you. If someone is nesting ternary expressions in order to distinguish three or more cases, I think the switch is generally going to be clearer. Writing `foo = ...` in each case, while it might seem redundant, is not really any worse than writing `return ...` in each case, sure. But I might very well use an explicit, separately written function if there's something obvious to call it. Just for the separation of concerns: working through the cases vs. doing something with the result of the case logic.

	▲	Scarblac 3 days ago \| parent [-]
		It just looked way more complex (and it's easy to miss the () at the end of the whole expression that makes it II). And the point of the rule was to make code more readable. Basically it's a shame that Typescript doesn't have a switch-style construct that is an expression. And that nowadays you can't make nested ternaries look obvious with formatting because automated formatters (that are great) undo it.

▲ 3 days ago | parent | prev [-]

[deleted]

▲ hamdingers 3 days ago | parent | prev | next [-]

> and the other side just pointed at the book

One of the most infuriating categories of engineers to work with is the one who's always citing books in code review. It's effectively effort amplification as a defense mechanism, now instead of having a discussion with you I have to go read a book first. No thanks.

I do not give a shit that this practice is in a book written by some well respected whoever, if you can't explain why you think it applies here then I'm not going to approve your PR.

	▲	rasmus-kirk 3 days ago \| parent [-]
		Yeah, and any of these philosophies are always terrible when you take them to their limit. The ideas are always good in principle and built on a nugget of truth, it's when people take it as gospel I have a problem. If they just read the book and drew inspiration for alternative, possibly better, coding styles and could argue their case that's unequivocally good.

▲ __s 4 days ago | parent | prev [-]

Cult think loves a tome