• Steve (unregistered)

    You've got an unclosed href after the hyperlink to Linus Torvald's latest effusions ...

  • (author)

    Something mashed a bunch of empty a tags into the document in no particular order. I assume my markdown processor must have done that.

  • (nodebb) in reply to Remy Porter

    I assume my markdown processor must have done that.

    I highlighted, or at least tried to, the key element in your problem. (And, indeed, the parenthetical comment is also indicative of that.)

  • Scott (unregistered)

    This is a wrong approach, not the wrong approach. There's a gazillion other ways to do this just as wrongly (or wrong in some new, interesting way) ;)

  • WTFGuy (unregistered)

    This smells strongly like there's not really a straightforward way to determine which sections were changed. So they first wrote a loop which found the changes via technique A. And then they realized there were exceptions where technique A had false negatives or positives (or better yet both). So they patched on technique B. Lather rinse repeat until somehow one of their n Techniques found all the changes in their test data. All tests green => push to Production. Hooray!

    One of the bad things about generics & fluent code style is it lets you write about 20 pages of code in a single screen. If you actually had to generate all the inner workings yourself, somewhere along the way you'd realize "Gosh, this is too stupidly complex to be the best way; I'd better fall back and start over with a clearer idea of what I'm trying to accomplish and what the underlying data tells me."

    Said another way ...

    Sometimes you have to code awhile down a blind alley to build up the detailed domain knowledge to understand how to design your solution for the business'es half-understood requirements vs. the API's half-documented behavior. The mistake comes once you've gained that knowledge and then you keep bulldozing down your exploratory code path trying to force your new knowledge into a junk design created in ignorance. You can often make that work sorta. And have your code immortalized here.

    Once you do understand the detailed domain space, far better to document your invariants and your goals in API-speak then start over. The solution that flows from that effort will be a lot better. Given the WTFery in many APIs it won't necessarily be good; but at least it'll be as good as it can be.

    Sometimes the rule "as simple as possible but no simpler" results in elegant beauty; other times it results in head-scratchers like quantum mechanics. But it always beats the alternative.

  • MiserableOldGit (unregistered) in reply to WTFGuy

    Couldn't agree more. Usually comes about because someone naively thought it would "just be a simple line or two of code" and then spent rather a long time discovering there was a lot more to it.

    As you say, as a method of discovery it's not necessarily a bad way (though it can be dangerous) but what you get is only a proof of concept, not shippable code.

    I have spent a lot of time in my career being handed atrocities like this "to just finish off" because "it's nearly there". Where I have managed to get the boss/client/friend with a problem to sit down and listen and accept that chunks of the code base need a proper rewrite based on what they've learned, and perhaps we need to build some regression tests, the projects have been very successful and led to more work. Those other projects, well I'm sure a few of them have found their way to this site, probably with some of my exasperated comments in there!

  • sizer99 (google)

    It's kind of nice that you've got two things you can see are instantly wrong without even trying to mentally parse the code.

    First the indentation, as noted. The shape immediately tells you it's wrong without even looking at the code.

    Then with nested generics you can immediately see it's wrong by the closing >>> (though I've seen much worse). If you get more than >> it's time to move it to structs.

    Yet they noticed nothing.

  • airdrik (unregistered)

    Well, seeing the use of java streams in there, I figure I'd have a go to see if it's any better if done entirely with streams:

    	return getDocSectionToSdSection().stream()
        .filter(tuple -> getVersionChanges().get(versionTag).entrySet().stream()
            .anyMatch(entry -> tuple._2.entrySet().stream()
                .filter(entry2 -> entry.getKey().startswith(entry2.getKey()))
                .anyMatch(entry2 -> entry.getValue().stream()
                    .anyMatch(change -> entry2.getValue().stream()
                        .anyMatch(lookFor -> change.startsWith(lookFor))
                    )
                )
            )
        ).map(tuple -> getDocSectionNumber(tuple._1))
        .sorted(Integer::compareTo)
        .collect(Collectors.toList());

    (note, I took some liberty of rearranging a couple of things, trying to keep the intent of the original, but this version doesn't have the "feature" where it will return the same section number multiple times if there were multiple matching versions)

    Yeah, not really any better with streams, but at least we get to make use of type inference to get rid of the types on the various intermediate variables.

  • Anon (unregistered)

    TRWTF is using 8 spaces for identation instead of 4.

  • gnasher729 (unregistered)

    Setting tabs to 4 spaces would be a huge improvement. Nowadays most of my code is Swift, and reading this I noticed how much more compact it is. Most of the type declarations would be unneeded. An array of integers is just [Int], a tuple is just (type1, type2) and can be directly assigned to two variables. Something.getValue() is just something.value. The code is just so much more readable - which may make inefficiencies much easier to spot.

  • Alan Scrivener (unregistered) in reply to WTFGuy

    "Sometimes you have to code awhile down a blind alley to build up the detailed domain knowledge to understand how to design your solution for the business'es half-understood requirements vs. the API's half-documented behavior. The mistake comes once you've gained that knowledge and then you keep bulldozing down your exploratory code path trying to force your new knowledge into a junk design created in ignorance. You can often make that work sorta."

    Genius! This is why most code sucks!

  • (nodebb)

    I find a poetic irony in how Linus's respectful disagreement of 80-column wrapping is itself wrapped by the mail system at 72 columns.

  • David Mårtensson (unregistered) in reply to WTFGuy

    Oh how I wished a former colleague would have grasped the concept of backtracking and starting over.

    He was more for the immutable principle, but not for variables but for code, once written, never change it, just wrap it and resolve any problems outside :/

    You do not want to see any examples, and I am never going to expose them to the world, just in case anyone thinks, "that might be a good way".

  • Anonymous Beaver (unregistered) in reply to airdrik

    The original won't do that either because it uses a set so all values will be unique.

  • Solw Purpose Of Visit (unregistered) in reply to airdrik

    You're too modest. It's much better with streams. In fact, the first thing I noticed (after the indentation) was that this was Java, using streams.

    I mean, I'd rather use Linq, but I'd hope that Java programmers understand the basics of streams by now. Much easier to read, much easier to think about, much easier to change mappings or whatever, and above all: programming at a higher level. Not a bunch of stupid nested if-elses.

  • (nodebb)

    Deeply nested for loops (or foreach) to not represent a code smell to me.... The mixing of "if" statements is (again to me) what adds the smell...

Leave a comment on “Try a Different Version”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article