3 ms·

Opened a random cpp source file, saw goto statements, and noped out of there real quick.

Cpoll 5y ago
https://github.com/nmrugg/stockfish.js/search?q=goto https://github.com/nmrugg/stockfish.js/search?q=goto Five occurrences of goto, for anyone that's curious. The code is pretty easy to read, good variable names, some explanatory comments. The gotos don't make spaghetti. I'm inclined to think the writers knew what they were doing, though goto offends my sensibilities as much as anyone. Could anyone with more experience explain the pattern? Why gotos for control flow along with >thousand line functions?
- mhink 5y ago
  Interesting. I was looking at the algorithm in evaluate.cpp [0] and it seems to be the case that the control flow is pretty much just an alternative to if-statements... which, personally, I find to be more readable than it'd otherwise be if it were heavily nested. I'm open to arguments otherwise, though. Consider two alternatives (written in pseudo-Javascript, I guess): let xyz = 0; if (!condition1) { doSomething(); xyz = getValueSomehow(); if (!condition2) { doSomethingElse(); xyz += getSomeOtherValueSomehow(); } } finishUpBusinessLogic(); return xyz; that seems much less readable than: let xyz = 0; if (condition1) goto done; doSomething(); xyz = getValueSomehow(); if (condition2) goto done; doSomethingElse(); xyz += getSomeOtherValueSomehow(); done: finishUpBusinessLogic(); return xyz; 0: https://github.com/nmrugg/stockfish.js/blob/d96c98872103c60b713e986901dd1e22986e9280/src/evaluate.cpp https://github.com/nmrugg/stockfish.js/blob/d96c98872103c60b...
  - kilburn 5y ago
    The hidden side-effects of your example code make it hard to reformat, but in general people don't advocate for your first example. The push is more towards defining smaller logic blocks (with names that hint at what they do): def computeSomething() { if (condition1) return 0; let value = getValueSomehow(); if (condition2) return value; doSomethingElse(); return value + getSomeOtherValue(); } let value = computeSomething(); finishUpBusinessLogic(); return value;
daniel-thompson 5y ago
There are a total of 5 gotos in the codebase¹. That's not bad for a chess engine which is heavily optimized for runtime performance above all other considerations. ¹ https://github.com/nmrugg/stockfish.js/search?q=goto&type= https://github.com/nmrugg/stockfish.js/search?q=goto&type=
Koffiepoeder 5y ago
Thinking in absolutes is never a good idea when it comes to a subjective matter (in this case: code style). goto statements can and sometimes do have good use cases, for instance finite state machines and error handling. It's not because something is generally not a recommended construct that you should never consider using it: in my opinion you should use all the tools you have in your belt and consider each use case separately to determine usefulness, practicality and other criteria applicable to your project.
ryeights 5y ago
IMO, although it is not used in this codebase, the “goto error” pattern for error handling is far more elegant and readable than any non-goto-using alternatives. Goto can have its place sometimes.
ComputerGuru 5y ago
Your loss.
eternityforest 5y ago
I can still appreciate it no matter how bad the code quality is, because it works amazingly well and I don't have to maintain it myself. If it works for the devs, then it's fine. Chess engines have a fairly low attack surface, all you have is moving pieces. Security is probably fine. Performance and strength are clearly fine.

https://github.com/nmrugg/stockfish.js/search?q=goto https://github.com/nmrugg/stockfish.js/search?q=goto Five occurrences of goto, for anyone that's curious. The code is pretty easy to read, good variable names, some explanatory comments. The gotos don't make spaghetti. I'm inclined to think the writers knew what they were doing, though goto offends my sensibilities as much as anyone. Could anyone with more experience explain the pattern? Why gotos for control flow along with >thousand line functions?

mhink 5y ago
Interesting. I was looking at the algorithm in evaluate.cpp [0] and it seems to be the case that the control flow is pretty much just an alternative to if-statements... which, personally, I find to be more readable than it'd otherwise be if it were heavily nested. I'm open to arguments otherwise, though. Consider two alternatives (written in pseudo-Javascript, I guess): let xyz = 0; if (!condition1) { doSomething(); xyz = getValueSomehow(); if (!condition2) { doSomethingElse(); xyz += getSomeOtherValueSomehow(); } } finishUpBusinessLogic(); return xyz; that seems much less readable than: let xyz = 0; if (condition1) goto done; doSomething(); xyz = getValueSomehow(); if (condition2) goto done; doSomethingElse(); xyz += getSomeOtherValueSomehow(); done: finishUpBusinessLogic(); return xyz; 0: https://github.com/nmrugg/stockfish.js/blob/d96c98872103c60b713e986901dd1e22986e9280/src/evaluate.cpp https://github.com/nmrugg/stockfish.js/blob/d96c98872103c60b...
- kilburn 5y ago
  The hidden side-effects of your example code make it hard to reformat, but in general people don't advocate for your first example. The push is more towards defining smaller logic blocks (with names that hint at what they do): def computeSomething() { if (condition1) return 0; let value = getValueSomehow(); if (condition2) return value; doSomethingElse(); return value + getSomeOtherValue(); } let value = computeSomething(); finishUpBusinessLogic(); return value;

daniel-thompson 5y ago

There are a total of 5 gotos in the codebase¹. That's not bad for a chess engine which is heavily optimized for runtime performance above all other considerations. ¹ https://github.com/nmrugg/stockfish.js/search?q=goto&type= https://github.com/nmrugg/stockfish.js/search?q=goto&type=

Koffiepoeder 5y ago

Thinking in absolutes is never a good idea when it comes to a subjective matter (in this case: code style). goto statements can and sometimes do have good use cases, for instance finite state machines and error handling. It's not because something is generally not a recommended construct that you should never consider using it: in my opinion you should use all the tools you have in your belt and consider each use case separately to determine usefulness, practicality and other criteria applicable to your project.

ryeights 5y ago

IMO, although it is not used in this codebase, the “goto error” pattern for error handling is far more elegant and readable than any non-goto-using alternatives. Goto can have its place sometimes.

ComputerGuru 5y ago

Your loss.

eternityforest 5y ago

I can still appreciate it no matter how bad the code quality is, because it works amazingly well and I don't have to maintain it myself. If it works for the devs, then it's fine. Chess engines have a fairly low attack surface, all you have is moving pieces. Security is probably fine. Performance and strength are clearly fine.