Prev: Java crash
Next: NullPointerException
From: markspace on 26 Feb 2010 13:52 Roedy Green wrote: > switch ( CSVSort.this.sortTypes[ i ] ) > { > case 's': Wow. You're an experienced programmer and the best you can come up with is to encode column types as a character? And then switch on it? That's really sad Roedy. Seriously, it's beneath anyone beyond a first year student. Why don't you make a real effort, then I'll comment. I'm not going to write your CSV class for you, and this feels like a vaguely transparent attempt to get someone to refactor your code for you. "Do your own homework." It's the only way for you to learn better programming habits. Here's a hint: There's lots of comments on this list about what to do if you are using a switch on types in an OO language like Java.
From: Lew on 26 Feb 2010 21:36 Roedy Green wrote: >>>> String no longer implements Comparable, it implements >>>> Comparable<String> which will not cast to Comparable or >>>> Comparable<Object> markspace wrote: >>> I'm pretty sure String will cast to Comparable<String>. Lew wrote: >> Of course it will. I don't know where the notion comes from that 'String' >> does not implement 'Comparable<String>', when it says right in the Javadocs >> that it does. Roedy Green wrote: > String DOES implement Comparable<String>. Nobody claimed it did not. I read markspace's comment as an implicit claim that it didn't. > The problem is when you want to use the Comparable the way you did > before generics, where you compare any two Objects and the JVM sorted > out which compareTo method to use depending on the type of the Object. But if you had different types in the column you'd get a run-time exception. That's bad. In the good new days, you use generics to catch that at compile time. Since, presumably, you know the type of the column's contents, you can make things play nicely before you reach run time. > You can't cast a Comparable<String> to a Comparable<Object> Nor should you want to in this case. If the column contains 'String' data, then you stay with 'Comparable<String>'. -- Lew
From: Steven Simpson on 27 Feb 2010 06:44
On 26/02/10 15:30, Alessio Stalla wrote: > On Feb 26, 12:50 pm, Roedy Green <see_webs...(a)mindprod.com.invalid> > wrote: > >> The columns must have COMPATIBLE types in >> them. e.g. all Strings, all Doubles, though different columns might >> have different types. >> >> I just don't want a big case clause to sort a column depending on the >> type that is currently in it. It is a reversion to C. >> >> Here some code from CSVSort that stinks. >> >> switch ( CSVSort.this.sortTypes[ i ] ) >> { >> case 's': // case sensitive, default compare >> String >> >> if ( CSVSort.this.isAscendings[ i ] ) >> > You're not forced to use that idiom. Rather than casting to concrete > types, cast to Comparable: > > if ( CSVSort.this.isAscendings[ i ] ) > { > /* ascending */ > diff = ( ( Comparable ) this.keys[ i ] > ).compareTo( other.keys[ i ] ); > } > else > { > /* descending */ > diff = ( ( Comparable ) this.keys[ i ] > ).compareTo( other.keys[ i ] ); > } > Furthermore, since the original code has the two arrays (sortTypes and isAscendings) from the outer class, why not just have one array of Comparator instead? The mapping from type specification ('s' to String, etc) would be done once in setting up that array, as would the ascend/descend order, and the loop would be even simpler. for (int i ...) { int diff = CSVSort.this.order[i].compare(this.keys[i], other.keys[i]); if (diff != 0) return diff; } You'd still have some 'unchecked' warnings to deal with, so I'm not offering anything better in that respect. -- ss at comp dot lancs dot ac dot uk |