• (nodebb)

    The frist thing that comes into my head is "Is this monstrosity actually called anywhere?". Or is it just a "joke" method?

  • Simon Clarkstone (unregistered)

    The line long randLong = randInt & 0xffffffffL; seems cargo-cult to me. It is a reasonable way to convert an int to a long without sign-extension, but that is irrelevant when the number can't be negative. Similarly the inner loop makes sense if the code was copied from somewhere that needed to output several distinct integers per line, but it's avoidable here.

  • Edd (unregistered)

    Gotta make sure the random int doesn't hoard all the randomness and leaves some for longs

  • Simon Clarkstone (unregistered)

    The line

    	long randLong = randInt & 0xffffffffL;

    seems cargo-cult to me. It is a reasonable way to convert an int to a long without sign-extension, but that is irrelevant when the number can't be negative. Similarly the inner loop makes sense if the code was copied from somewhere that needed to output several distinct integers per line, but it's avoidable here.

  • TruePony (unregistered)

    Ah, TRWTF is that the ArrayList is unsorted, so they have to do a linear search instead of a binary search to check for duplicates.

  • Prime Mover (unregistered)

    This always annoys me:

    ArrayList<BigDecimal> numbers = new ArrayList<BigDecimal>();

    Should at least be:

    List<BigDecimal> numbers = new ArrayList<BigDecimal>();

    or even:

    Collection<BigDecimal> numbers = new ArrayList<BigDecimal>();

    Doesn't really matter here, of course, but if you're not in the habit of automatically implementing your objects as interfaces, you're clearly not going far.

  • akozakie (unregistered) in reply to Prime Mover

    Why? We had one "programmer" who always wrote like this, using the interfaces only when there was no other choice. That was one of his many... oddities.

    He certainly went far! Far, far away. Very quickly.

  • LegacyWarrior (unregistered)

    To me, this reads like the possible results of poorly defined or a long and slow narrowing of business requirements. "It should print any number." "ok." "No, it should only print a single digit number." "ok." "No, you know I mean that doesn't include zero." "ok." etc, etc, etc. And the crappy developer decided to just say 0xffffffffL it and kept lazy-patching things as they knew the specs would keep changing... until they didn't.

  • Nobody (unregistered) in reply to Prime Mover

    I would prefer this:

    List<BigDecimal> numbers = new ArrayList<>();

  • (nodebb) in reply to Simon Clarkstone

    The line

    long randLong = randInt & 0xffffffffL;

    seems cargo-cult to me

    It's an attempt to avoid using Integer.toUnsignedLong().

  • while(0 != x = rand(comments.length)) ; (unregistered)

    Is this code actually wrong? I think it will generate the correct output. It does it in a really terrible way, but being at least functional is a step above what a lot of people manage.

    That bit mask is a reasonable thing to do if you're casting a logically unsigned value to a larger signed type, and the language doesn't really support that. I've done the same for byte->int in Java before. But since int is signed and the values are all positive, it's completely pointless here, indeed.

  • (nodebb) in reply to while(0 != x = rand(comments.length)) ;

    What's worse, non-functional code, or functional but terrible code whose author then goes on to write more important code just as terribly?

  • (author)

    Obviously the worst Real WTF is that the string ends up with a trailing space.

  • WTFGuy (unregistered)

    IANA Java dev but a quick Google suggests the .WriteLine() method does what we'd expect from other languages, namely output its argument(s) followed by a newline token of however many char(s) is appropriate for that OS/environment.

    So the code doesn't exactly implement the spec we're given: "...generate some number of random numbers between 1 and 9, and pass them off to a space-separated file."

    Instead it passes them off to a newline-delimited file with a blank char on the end of each line after each number and before the newline.

    I have to say Aurelia's coworker(s) seem like the best fount of real headscratcher WTFs we've seen here in awhile. I for one want to encourage her to submit more!

    As a next installment I think it'll be fun for Aurelia to locate the code that consumes this file and see how it works to filter out the undesired newlines. Bonus points if the producing and consuming environments have different newlines where the difference is either ignored or handled in the dumbest possible way.

  • SyntaxError (unregistered) in reply to Prime Mover

    Id use the form that most closely matches the use intent...

    Id use "List<BigDecimal> numbers = ..." when it is necessary to know that the collection is ordered in some fashion or there is the need for indexed lookup. Id use "Collection<BigDecimal> numbers = ..." when the general case is to just iterate over the full collection.

    With parameters, I try to be as accommodating as possible (aka accepting Collection<...>) and when returning, I tend to use the most specific interface just short of the implementation (aka returning List<...>, Set<...>, or Collection<...> if it could be either and the general use case is iterating).

    However, my statements above don't really apply to the code at hand (not accepting or returning a list, but using a local variable), I would use whatever interface that best communicates the intent. The algorithm depends on order or indexed lookups? List. The algorithm depends on unique elements? Set. It doesn't really care and just needs to iterate? Collection.

    The code in the article does not do a good job of communicating the intent at all. It is not even in the ball park (and no, not because its a home run).

  • Appalled (unregistered) in reply to while(0 != x = rand(comments.length)) ;

    I'd suggest you are correct. Starting with an ill-defined spec, and then mangling the first pure attempt by Googling up "how to"s and applying them until the damn thing finally spits out exactly what was requested. The programmer was either new to programming or new to the language. But whatever, job done. Except it wasn't. I can't imagine leaving something like that to posterity (myself included). Now would be the time to refactor/simplify by removing superfluous crap and/or research the language for other simpler methods. At least the specs are finally solid.

  • James (unregistered)

    goto considered harmful, they said . . .

  • (nodebb)

    I wonder if the original intention was that all the numbers should be different? The developer copied something off StackOverflow for generating a list of distinct numbers, tweaked it because the original generated numbers in the range 0..Long::MAX_VALUE. And after some thrashing around was then surprised by the way it ended up in an infinite loop every time more than nine numbers were requested.

    Told that "duplicates are okay," the developer did the minimum amount of needful and moved the List initialiser down a couple of lines.

  • Simon (unregistered) in reply to Watson

    Well, if I wanted to make sure I had distinct values, I'd put the range of possible values in an array, shuffle them, and draw as many as I needed from the start of the array. (Although this would be somewhat memory-inefficient if the number of possible values is far greater than the number drawn.) In the C++ Standard Library there is std::shuffle, IANA Java pogrommer but I believe there is Collections.shuffle().

  • Dave (unregistered)

    The Real WTF is that this code doesn't call "Randomize()", so it will generate the same list of numbers, in the same order, every time the application is run.

  • Aurelia (unregistered)

    OP here. Found some more context. Basically, there was a need to generate random sets of numbers. Something like 5 unique, random numbers out of 20. Or 10 numbers out of 100. Hence the ArrayList, and the check if the number already exists, as well as the while (numbers.size() < x). Each group of numbers needs to go on a separate line.

    It would not surprise you to learn that they did not create one method that takes a number of picks and range as parameters. Instead, this method has been copy-pasted several times, with tweaks to hard-coded variables. In one case (this one), only one number out of 10 (well, 1 to 9) was needed.

    I can't help but feel that this clarification removes some of the WTF, and introduces a whole lot more.

Leave a comment on “A Random While”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article