5 ms·

https://github.com/dhewm/dhewm3/blob/master/neo/game/Target.cpp#L964 https://github.com/dhewm/dhewm3/blob/master/neo/game/Target.... This is the "nicest lookin

• toxicFork 12y ago
  What's wrong with this?
  • golergka 12y ago
    Over 150 lines of code in a simple method? C-style declaration of all the variables in the beginning? (Yes, I know what Carmack said about how he written the project; this explains it, but doesn't make it good C++ code.) Unclear flow with just one comment, which doesn't really explain anything?
    • zimpenfish 12y ago
      You should try a Perl codebase. In just this one module I'm looking at, there's methods with 505, 880, and 995 lines.
      • MichaelGG 12y ago
        A popular VoIP product has switch statements that are in the thousands.
        • EpicEng 12y ago
          Is it automatically generated?
          • MichaelGG 12y ago
            No, hand written.
    • Demiurge 12y ago
      Why do you say it's a simple method, if it is 150 lines long? Are some lines not necessary? There is a logic to keeping logic together if it is isolated: http://number-none.com/blow/john_carmack_on_inlined_code.html http://number-none.com/blow/john_carmack_on_inlined_code.htm... Context is important, to anyone developing D3 code, this code might be obvious and comments would be unnecessary. Considering 'geterdone' attitude of many game projects, with high levels hackery, this is very nice code.
      • golergka 12y ago
        Thanks for the link, it's a very fascinating read! By the way, I discussed this issue on Programmers stack exchange a while back, you might find this interesting: http://programmers.stackexchange.com/questions/277098/method-extraction-vs-underlying-assumptions http://programmers.stackexchange.com/questions/277098/method...
    • angersock 12y ago
      It's a "simple method", but it's responsible for a bunch of initialization, so yeah, it's kind of expected to be long. Notice, though, that each section is pretty straightforward in what it does...basically forEach's. You could make the argument for greater polymorphism in types, or something like a visitor pattern over collections or something, but honestly this is readable and maintainable code. More comments at the function declaration would've been nice, but the implementation is pretty straightforward.
    • anon4 12y ago
      I found no difficulty understanding what it does. Perhaps you need to acquire greater proficiency at reading code. Well, some familiarity with the game helps, too. This is the code that runs when a certain pick-up item's effect activates.
• edc117 12y ago
  Just curious, are you cherry picking, or are there legitimately many areas of the code like this? I haven't had a chance to look yet.
  • Toine 12y ago
    To be honest i didn't look at all the code, but yes there are many areas like this.
• dman 12y ago
  Any critiques in specific?
  • CamperBob2 12y ago
    if ( ent->GetRenderEntity()->gui[ j ] && ent->spawnArgs.FindKey( j == 0 ? "gui_demonic" : va( "gui_demonic%d", j+1 ) ) ) { ent->GetRenderEntity()->gui[ j ] = uiManager->FindGui( ent->spawnArgs.GetString( j == 0 ? "gui_demonic" : va( "gui_demonic%d", j+1 ) ), true ); Really?
    • dman 12y ago
      Fair enough.