7 ms·

14 levels of indentation across 811 lines. Wow indeed. I refactor as soon as my code gets past 3 levels, I don't know how one is supposed to readily understand

14 levels of indentation across 811 lines. Wow indeed. I refactor as soon as my code gets past 3 levels, I don't know how one is supposed to readily understand this block of code. The lack of comments isn't what troubles me. Comments get old and, when they do, they get completely misleading and downright dangerous. As soon as you feel the need to add a comment, it's often because that particular chunk of code should be extracted into its own function/method/whatever and given an intelligent name. Comments should be reserved for really important stuff, like when the code does something totally non-obvious.

saiya-jin 10y ago
i think they don't use static code analysis on this part :) generally agree on commenting, but it's also about discipline - method-level commands are rarely updated, but if there is some particular algorithm within method, I don't see a reason to describe it beforehand on a line or two. General rule - if method is longer than my screen, it's time to refactor. very rarely this isn't a good idea.
- shermanyo 10y ago
  maybe the author got a 4k monitor and set it up in portrait? :P
webkike 10y ago
This code is not actually that _complex_, it is just _complicated_, and there are probably several people within the company that understand it completely. If you actually go through the code, I bet you will understand the majority of it. Step functions are annoying and complicated and need to be _fast_, less than half a frame long. And you need to do a lot in them, and all of those things are tightly coupled.
- damptowel 10y ago
  another such beauty http://hg.icculus.org/icculus/lugaru/file/97b303e79826/Source/GameTick.cpp#l7276 http://hg.icculus.org/icculus/lugaru/file/97b303e79826/Sourc...
  - zubspace 10y ago
    Wow. Question regarding index access: There are tons of for loops with player[k] occurrences and similar. Personally I would create a local variable and prevent further index-retrievals. Is this worthwhile or would a compiler optimize the array-index access away? Or maybe subsequent uses of the same index are cached by the cpu? Does someone know?
    - tehrei 10y ago
      That's a micro-optimisation that's absolutely worthless. Even if you do pay the cost of indexing an array every single time, that's just a single instruction to fetch the address. The compiler is absolutely capable of caching the indexing for you, either in a register or on the stack, if it deems it worthwhile. One reason you might want to use a separate variable would be to prevent indexing mistakes, like using i instead of k by mistake in a context where i is a nested loop's counter. But apart from this and other "to make it easier on the human" reasons, there's no point in caching that in a variable. Now, if you were writing Python on the other hand...
  - ensiferum 10y ago
    Holy batman. Now this is officially the worst piece of code I've ever seen in my 15 years of programming. This has to be generated... right?? I can't believe any human wrote this. Grabs the eye bleach
    - webkike 10y ago
      It was written by a highschooler who made a 3D video game with decent physics and you're... criticizing him? Man, I'd like to see what YOU wrote in highschool.
- PeCaN 10y ago
  On top of that, in a step function you really want to see what's being modified (that's kinda the point...). I suspect that monstrosity is easier to maintain than having state updates scattered around and having to jump to definitions and back to step() to understand the thing.
clux 10y ago
There are also 3 gotos in that function.
- khedoros 10y ago
  Goto should be used in the (admittedly rare) cases where it makes program flow cleaner. The original complaint was that people weren't moving to structured programming constructs as they became available. That really isn't a problem any more.

saiya-jin 10y ago

i think they don't use static code analysis on this part :) generally agree on commenting, but it's also about discipline - method-level commands are rarely updated, but if there is some particular algorithm within method, I don't see a reason to describe it beforehand on a line or two. General rule - if method is longer than my screen, it's time to refactor. very rarely this isn't a good idea.

shermanyo 10y ago
maybe the author got a 4k monitor and set it up in portrait? :P

webkike 10y ago

This code is not actually that _complex_, it is just _complicated_, and there are probably several people within the company that understand it completely. If you actually go through the code, I bet you will understand the majority of it. Step functions are annoying and complicated and need to be _fast_, less than half a frame long. And you need to do a lot in them, and all of those things are tightly coupled.

damptowel 10y ago
another such beauty http://hg.icculus.org/icculus/lugaru/file/97b303e79826/Source/GameTick.cpp#l7276 http://hg.icculus.org/icculus/lugaru/file/97b303e79826/Sourc...
- zubspace 10y ago
  Wow. Question regarding index access: There are tons of for loops with player[k] occurrences and similar. Personally I would create a local variable and prevent further index-retrievals. Is this worthwhile or would a compiler optimize the array-index access away? Or maybe subsequent uses of the same index are cached by the cpu? Does someone know?
  - tehrei 10y ago
    That's a micro-optimisation that's absolutely worthless. Even if you do pay the cost of indexing an array every single time, that's just a single instruction to fetch the address. The compiler is absolutely capable of caching the indexing for you, either in a register or on the stack, if it deems it worthwhile. One reason you might want to use a separate variable would be to prevent indexing mistakes, like using i instead of k by mistake in a context where i is a nested loop's counter. But apart from this and other "to make it easier on the human" reasons, there's no point in caching that in a variable. Now, if you were writing Python on the other hand...
- ensiferum 10y ago
  Holy batman. Now this is officially the worst piece of code I've ever seen in my 15 years of programming. This has to be generated... right?? I can't believe any human wrote this. Grabs the eye bleach
  - webkike 10y ago
    It was written by a highschooler who made a 3D video game with decent physics and you're... criticizing him? Man, I'd like to see what YOU wrote in highschool.
PeCaN 10y ago
On top of that, in a step function you really want to see what's being modified (that's kinda the point...). I suspect that monstrosity is easier to maintain than having state updates scattered around and having to jump to definitions and back to step() to understand the thing.

clux 10y ago

There are also 3 gotos in that function.

khedoros 10y ago
Goto should be used in the (admittedly rare) cases where it makes program flow cleaner. The original complaint was that people weren't moving to structured programming constructs as they became available. That really isn't a problem any more.