• EuroGuy (unregistered)

    This is close to how one would do this in more lower level languages such as C. So whoever made this probably recently started using Java and used his experiences from previous jobs . Sure it could have been done more efficient, but it works. So no real WTF here I say.

  • (cs)

    Even if the library didn't offer methods for splitting strings, copying array contents, or building a growing list, there's still no good reason to initialise ret to a 0-element array and have an extra array-extend operation outside the loop. That's the WTF cherry on top.

  • (cs)

    The inability of some people (I refuse to use the phrase 'developers') to read a simple man page baffles me.

    Another thing is not moving with times. If they used a Vector in the early 2000's, they will continue using it even though Sun/Oracle have been telling us for a decade that we really should be using List instead.

    Yes, the Java standard library is quite large, but just reading the JavaDocs once in a while won't kill anybody.

  • (cs) in reply to pjt33
    pjt33:
    there's still no good reason to initialise ret to a 0-element array and have an extra array-extend operation outside the loop. That's the WTF cherry on top.
    Furthermore, this part
    ret = new String[temp.length]; for (int j=0; j < ret.length; j++) { ret[j] = temp[j]; }
    could have been replaced with "ret = temp", because the value of temp is not used again after that. Also, instead of copying by hand or using the quite complex System.arraycopy(), as the article suggests, the following would also have done the job:
    ret = Arrays.copyOf(ret, ret.length + 1);

    That's the WTF whipped cream and chocolate crumbles on top.

  • Pista (unregistered) in reply to EuroGuy
    EuroGuy:
    This is close to how one would do this in more lower level languages such as C. So whoever made this probably recently started using Java and used his experiences from previous jobs . Sure it could have been done more efficient, but it works. So no real WTF here I say.

    If this is what happened, then we can say that the guy didn't read the f...ing manual, which is quite a WTF, IMHO.

  • A C (unregistered)

    What's wrong with using StringTokenizer (since v1.0)?

  • A C (unregistered) in reply to Severity One

    And which class would you instantiate to create this List?

  • (cs) in reply to A C
    And which class would you instantiate to create this List?

    I am no Java guy but maybe ArrayList.

    If I remember correctly then the main difference is that Vector is synchronized, so whichever you want to use depends on the situation.

  • (cs)

    Given that the programmer creates a new ret and copies temp back into it, rather than just doing "ret=temp;", makes it clear that the programmer also has no concept of objects or pointers

    Whoa, get your high-water pants on: It's an array flood!

  • Smug Unix User (unregistered)

    Five minutes on Google saves five hours of analyzing the "custom" solution and the differences before refactoring to use the built in way (sometimes with additional changes to make it work how it did before).

  • HandsomeStan (unregistered) in reply to Coyne
    Coyne:
    Given that the programmer creates a new ret and copies temp back into it, rather than just doing "ret=temp;", makes it clear that the programmer also has no concept of objects or pointers

    Whoa, get your high-water pants on: It's an array flood!

    Haha, Java guys complaining that it's C guys who have no understanding of pointers.

    I would say that this guy has been working with unmanaged memory in C-like environments for far too long. Combine that with some naivety regarding Java object handling and it's clear what kind of intentions spawned this mess.

  • iWantToKeepAnon (unregistered)

    java;dr

  • (cs) in reply to ben_wtf
    ben_wtf:
    If I remember correctly then the main difference is that Vector is synchronized, so whichever you want to use depends on the situation.
    The standard advice pretty much since Java 2.0 is that if you want a synchronised ArrayList you should use Collections.synchronizedList to wrap an ArrayList. The only reason for using Vector in the past decade+ is that some APIs (looking at you, Swing) haven't been updated.
  • A C (unregistered) in reply to pjt33

    Why do that when Vector is already synchronized? Isn't it the simplest class that implements List? I wouldn't be surprised if the internal Vector code does use ArrayList.

  • Daniel (unregistered)

    See also: StringTokenizer

  • Daniel (unregistered) in reply to A C

    Nothing, although the article author is unaware that StringTokenizer existed since JDK1.0 (according to the javadoc).

  • smk (unregistered)

    This is C code literally translated to Java. Nothing to see here... Move along...

  • Davio (unregistered)

    When it takes more than 20 lines to do something fairly simple, you're probably doing it wrong.

  • Charles F. (unregistered)

    Of course, there are the "up to date" Java programmers who try to split a pipe-delimited String like this:

    String[] notTheResultYouWant = pipeDelimitedString.split("|");

    Good times, good times.

  • (cs) in reply to A C
    A C:
    Why do that when Vector is already synchronized? Isn't it the simplest class that implements List?
    To answer the second question first, no, that's ArrayList. Vector predates the Java Collection Framework but was retrofitted to its List API, so it has that API and an older one which it doesn't share with any other class or interface.

    To answer the first question: there are at least two reasons. Firstly, Vector is synchronised on itself: this is a bad practice; Collections.synchronizedList returns a list which is synchronised on a privately stored object, preventing idiots from synchronising on the list and creating lock conflicts.

    Secondly, if you decide that you want to use a different underlying implementation (e.g. LinkedList) it's the difference between changing one contructor call (s/ArrayList/LinkedList/) and having to ask "Was this using Vector because it needed to be synchronised, in which case I need to add a call to Collections.synchronizedList, or was it for some other reason?"

    (Back when people believed that Sun might one day actually remove anything from the API - which is what "Deprecated" was originally intended to signal - there may also have been a belief that Vector would be deprecated and removed and you were better off using ArrayList to ensure forward compatibility).

  • cyborg (unregistered) in reply to Charles F.
    Charles F.:
    Of course, there are the "up to date" Java programmers who try to split a pipe-delimited String like this:

    String[] notTheResultYouWant = pipeDelimitedString.split("|");

    Good times, good times.

    One does on occassion forget that they are regular expressions.

  • (cs)

    Its clear that a lot of people claiming to be java developer needs the right mentoring to create the right code.

    I have seen MANY SUCH WTF coding practices in our code review, but I have gone above and beyond to fix them.

  • amomynous (unregistered)

    Java guy here.

    Crappy overcomplicated code by some dumbass that do not RTFM. Not going to waste brainpower trying to understand that.

  • djingis1 (unregistered) in reply to EuroGuy
    EuroGuy:
    This is close to how one would do this in more lower level languages such as C. So whoever made this probably recently started using Java and used his experiences from previous jobs . Sure it could have been done more efficient, but it works. So no real WTF here I say.
    If this person used their experience from a language like C, they should be kept as far away from a compiler as possible.

    There is no reason whatsoever to keep copying the temp array into a new one of size +1. Instead, loop over the string twice. Once to determine the amount of tokens. Then initialize the array. Then loop over the string a second time to put the tokens in the array. That changes an O(N^2) algorithm to O(N).

    So yes, even if this person came from a C-like background, this is very much a WTF.

  • amomynous (unregistered) in reply to djingis1
    djingis1:
    If this person used their experience from a language like C, they should be kept as far away from a compiler as possible.
    Last time I checked, C needs a compiler. But yeah, I agree...
  • (cs) in reply to iWantToKeepAnon
    iWantToKeepAnon:
    java;dr

    If this discussion had occurred on the Discourse forum, I would have <3'ed this.

  • (cs) in reply to Captain Oblivious
    Captain Oblivious:
    iWantToKeepAnon:
    java;dr

    If this discussion had occurred on the Discourse forum, I would have <3'ed this.

    If this discussion had occurred on the Discourse forum, I would have !<3 'ed the Discourse page layout.

  • Boulder (unregistered)

    The chocolate syrup drizzled on top of the whipped cream and chocolate crumbles is that the final copy of temp to ret also adds one last element and puts the full original string into it.

  • (cs)

    Bad Java code....you don't say...

  • (cs) in reply to Smug Unix User
    Smug Unix User:
    Five minutes on Google saves five hours of analyzing the "custom" solution and the differences before refactoring to use the built in way (sometimes with additional changes to make it work how it did before).

    Some people don't have access to Google. You should know this.

    On a complete unrelated note, The Queen's Horse is a a user.

  • (cs) in reply to Nagesh
    Nagesh:
    Smug Unix User:
    Five minutes on Google saves five hours of analyzing the "custom" solution and the differences before refactoring to use the built in way (sometimes with additional changes to make it work how it did before).

    Some people don't have access to Google. You should know this.

    On a complete unrelated note, The Queen's Horse is a a user.

    Answer: Poppy Seeds.

  • (cs) in reply to iWantToKeepAnon
    iWantToKeepAnon:
    java;dr
    +1!!
  • Davio (unregistered) in reply to Charles F.
    Charles F.:
    Of course, there are the "up to date" Java programmers who try to split a pipe-delimited String like this:

    String[] notTheResultYouWant = pipeDelimitedString.split("|");

    Good times, good times.

    Gotta love the super sneaky regex splitting.

  • (cs) in reply to herby
    herby:
    Nagesh:
    Smug Unix User:
    Five minutes on Google saves five hours of analyzing the "custom" solution and the differences before refactoring to use the built in way (sometimes with additional changes to make it work how it did before).

    Some people don't have access to Google. You should know this.

    On a complete unrelated note, The Queen's Horse is a a user.

    Answer: Poppy Seeds.

    Wrong Answer, Gungadin!

  • (cs) in reply to HandsomeStan
    HandsomeStan:
    Coyne:
    Given that the programmer creates a new ret and copies temp back into it, rather than just doing "ret=temp;", makes it clear that the programmer also has no concept of objects or pointers

    Whoa, get your high-water pants on: It's an array flood!

    Haha, Java guys complaining that it's C guys who have no understanding of pointers.

    I would say that this guy has been working with unmanaged memory in C-like environments for far too long. Combine that with some naivety regarding Java object handling and it's clear what kind of intentions spawned this mess.

    Total red herring. The code is wrong even in C with unmanaged memory. In that environment, temp and ret would both have to be pointers for this code to work at all, and it is completely idiotic to allocate a new array for ret and copy temp to ret, when a simple = would do. After all, during the next loop, he is generating a new temp, so there will be no conflict.

    I'd suggest this was COBOL programmer stuff, but this is below even sensible COBOL code.

  • (cs) in reply to Charles F.
    Charles F.:
    Of course, there are the "up to date" Java programmers who try to split a pipe-delimited String like this:

    String[] notTheResultYouWant = pipeDelimitedString.split("|");

    Good times, good times.

    No big deal. The first time you run your test data through it you realise you've made a silly mistake and so you read the API a little more closely. Oh yes, you think, I forgot the argument has to be a regexp. And you correct your code and carry on. If, of course, you just put this line into your program and don't check it, then I'm afraid you're a bit of a cunt.

  • Norman Diamond (unregistered) in reply to Charles F.
    Charles F.:
    Of course, there are the "up to date" Java programmers who try to split a pipe-delimited String like this:

    String[] notTheResultYouWant = pipeDelimitedString.split("|");

    Good times, good times.

    Some people, faced with a problem, say "I know! I won't try to use regular expressions; I'll just assume they don't exist." Then they have two problems.

  • Norman Diamond (unregistered) in reply to smk
    smk:
    This is C code literally translated to Java.
    The WTFs would be just as bad in C. If the article had this code in C, someone just like you would say that it was Java code literally translated to C.

    I wrote code in Fortran that had some of these WTFs. Arcane constructions were unavoidable because Fortran IV grudgingly provided just enough support for strings to let them exist, but I made these kinds of logic WTFs and overly complex loops because I was 16 years old, in first year of university, and prior experience had not included analysis of algorithms.

  • Martin Cohen (unregistered) in reply to Nagesh

    If you are like me, you are the only one in the code review who actually read the code.

  • Eff Jatwood (unregistered)

    My bet is on this coming from someone with VB background and a very very small hint of C.

  • Norman Diamond (unregistered) in reply to Eff Jatwood
    Eff Jatwood:
    My bet is on this coming from someone with VB background and a very very small hint of C.
    Most of the WTFs would be just as bad in VB.

    Someone remind me to save a USB floppy drive. I'll need it to install VB 1 for testing.

  • Lrr (unregistered)

    Code like this should be a firing offence. It impressively presents a level of incompetence that is not easily remedied even by training; so the company should just let the employee go, and soon. Just sayin'.

  • (cs) in reply to Eff Jatwood
    Eff Jatwood:
    My bet is on this coming from someone with VB background and a very very small hint of C.

    Yeah, because the VB to Java transition is common.

  • Norman Diamond (unregistered) in reply to Lrr
    Lrr:
    Code like this should be a firing offence. It impressively presents a level of incompetence that is not easily remedied even by training; so the company should just let the employee go, and soon. Just sayin'.
    It is remedied by training if the programmer was bright but had zero training before this. If the person had a college degree or a previous programming job and still put out this garbage, they're hopeless and need to be promoted to manage programmers.
  • (cs) in reply to pjt33
    pjt33:
    To answer the first question: there are at least two reasons. Firstly, Vector is synchronised on itself: this is a bad practice; Collections.synchronizedList returns a list which is synchronised on a privately stored object, preventing idiots from synchronising on the list and creating lock conflicts.
    No, the mutex object is usually the synchronizedList wrapper itself. You need this because you still have to deal with maintaining consistency across compound operations. The problem with Vector was that people were buttuming that they didn't need to do this sort of thing, which was BS, and that lots of code which provably had no threading issues at all (because it was all working with local variables in a method rather than class or instance variables) was still having to pay the cost of synchronisation.
  • Mark (unregistered) in reply to amomynous
    amomynous:
    djingis1:
    If this person used their experience from a language like C, they should be kept as far away from a compiler as possible.
    Last time I checked, C needs a compiler. But yeah, I agree...
    I think that's the OP's point.
  • AWO EGh (unregistered)

    String Tokenizer might not cut it (no pun intended).....I notice the delimiter is a String (I think Java's StringTokenizer Tokenizes on ANY element in the delimiter string, this appears to try to tokenize on the WHOLE string....but I'm not Javanese) That is, Java's ST takes delimiters whereas this one takes a string delimiter....

    http://docs.oracle.com/javase/7/docs/api/java/util/StringTokenizer.html#StringTokenizer(java.lang.String,%20java.lang.String)

    Loads of other WTF there, but not using the strtok is not one of them.

  • trtrwtf (unregistered) in reply to cellocgw
    cellocgw:
    Captain Oblivious:
    iWantToKeepAnon:
    java;dr

    If this discussion had occurred on the Discourse forum, I would have <3'ed this.

    If this discussion had occurred on the Discourse forum, I would have !<3 'ed the Discourse page layout.

    If this discussion had occurred on the Discourse forum, I wouldn't have seen it.

  • turist (unregistered) in reply to smk
    smk:
    This is C code literally translated to Java. Nothing to see here... Move along...

    If this is C code, the coder sucks in C, too. First, there would be the obvious use of strtok(). And in case you don't want to modify your "toSplit" string, you'd do something like this:

    char **
split(char *toSplit, char *delim)
{
    size_t delimSize = strlen(delim);
    size_t maxElements = strlen(toSplit) / delimSize + 2;
    size_t elements = 0;
    char **result = calloc(maxElements, sizeof(char *));
    char *work = toSplit;

    char *nextdelim;
    while (*work)
    {
        nextdelim = strstr(work, delim);
        if (nextdelim)
        {
            size_t elementSize = nextdelim - work;
            char *element = calloc(elementSize+1, sizeof(char));
            memcpy(element, work, elementSize);
            result[elements++] = element;
            work += elementSize + delimSize;
        }
        else
        {
            char *element = calloc(strlen(work)+1, sizeof(char));
            memcpy(element, work, strlen(work));
            result[elements++] = element;
            break;
        }
    }
    result[elements++] = 0;
    return realloc(result, elements * sizeof(char *));
}
  • Norman Diamond (unregistered) in reply to trtrwtf
    trtrwtf:
    cellocgw:
    Captain Oblivious:
    iWantToKeepAnon:
    java;dr
    If this discussion had occurred on the Discourse forum, I would have <3'ed this.
    If this discussion had occurred on the Discourse forum, I would have !<3 'ed the Discourse page layout.
    If this discussion had occurred on the Discourse forum, I wouldn't have seen it.
    Autists don't <3 discourse.

Leave a comment on “Doing a Split...the Hard Way”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article