36

I was recently trying to convert a string literal into a boolean, when the method boolean Boolean.getBoolean(String name) popped out of the auto-complete window. There was also another method (boolean Boolean.parseBoolean(String s)) appearing right after, which led me to search to find out what were the differences between these two, as they both seemed to do the same.

It turns out that what Boolean.getBoolean(String name) really does is to check if there exists a System property (!) of the given name and if its value is true. I think this is very misleading, as I'm definitely not expecting that a method of Boolean is actually making a call to System.getProperty, and just by looking at the method signature, it sure looks (at least to me) like it should be used to parse a String as a boolean. Sure, the javadoc states it clearly, but I still think the method has a misleading name and it is not in the right place. Other primitive type wrappers, such as Integer also have a similar method.

Also, it doesn't seem to be a very useful method to belong in the base API, as I think it's not very common to have something like -Darg=true. Maybe it's a good question for a Java position interview: "What is the output of Boolean.getBoolean("true")?". I believe a more appropriate place for those methods would be in the System class, e.g., getPropertyAsBoolean; but again, I still think it's unnecessary to have these methods in the base API. It'd make sense to have them in something like the Properties class, where it's very common to do this type of conversions.

What do you think of all this? Also, if there's another "awkward" method that you're aware of, please post it.

N.B. I know I can use Boolean.valueOf or Boolean.parseBoolean to convert a string literal into a boolean, but I'm just looking to discuss the API design.

João Silva
  • 89,303
  • 29
  • 152
  • 158
  • 1
    Wow, that *is* awkward. I hope they fired whoever came up with that one :P – Thorarin Aug 18 '09 at 05:55
  • 2
    More experienced Java developers are unlikely to get confused about this, since they'd know that valueOf() is a convention used consistently for all value classes. – Michael Borgwardt Aug 18 '09 at 06:11
  • It seems to me you shouldn't have to be a more experienced Java developer to avoid being tripped up by such a method... – Steve McLeod Aug 19 '09 at 07:28
  • 2
    Note that many of these seemingly dumb API method names date from the earliest days of Java, before many of today's conventions were established. For backward-compatibility reasons these dumb methods have to stay. – Steve McLeod Aug 19 '09 at 07:36
  • 1
    But they can deprecate this method, and then remove it in next Java version. – Amir Pashazadeh Mar 17 '12 at 04:42

17 Answers17

43

The URL equals() method compares IP addresses, uses a network connection and is a blocking operation!

From the javadocs:

Two hosts are considered equivalent if both host names can be resolved into the same IP addresses; else if either host name can't be resolved, the host names must be equal without regard to case; or both host names equal to null.

Since hosts comparison requires name resolution, this operation is a blocking operation.

Note: The defined behavior for equals is known to be inconsistent with virtual hosting in HTTP.

Use URI instead.

Daniel Rikowski
  • 71,375
  • 57
  • 251
  • 329
dogbane
  • 266,786
  • 75
  • 396
  • 414
  • I was about to mention this one. What a nutty api function. Joshua Bloch highlighted this "feature" in one of this talks. – Berlin Brown Mar 29 '11 at 16:37
32

One well-known problem with the Calendar class is that months are numbered 0 to 11 instead of 1 to 12. It's very easy to make a mistake like this:

Calendar cal = Calendar.getInstance();

// Set date to August 18, 2009? WRONG! Sets the date to September 18, 2009!
cal.set(2009, 8, 18);

The right way to do it is by using constants for the months:

cal.set(2009, Calendar.AUGUST, 18);

But the method makes it much too easy to make the mistake of using the normal month numbers 1 to 12.

I regard this as a mistake in the design of the Calendar class.

Jesper
  • 202,709
  • 46
  • 318
  • 350
  • It was designed that way so that you could look up, in a standard 0-based array, what the name of a month was given its number. Besides, everybody uses the constants, don't they? :-P – Jonathan Aug 18 '09 at 14:18
  • 3
    this was designed before enums – Jason S Aug 18 '09 at 17:03
  • 5
    If the Calendar class provide the lookup facility (With I18N support, of course) then the zero based farce would never have been necessary in the first place. Anyway, most programmers are used to removing or adding one, in order to deal with indexes but - at the end of the day - a month is an ordinal value, starting at one, and this convention has been around for so long it was a very bad decision to ignore the convention for such a pathetic reason. This is made even more annoying by the fact that the DAY_OF_WEEK values go from 1 to 7!!! – belugabob Aug 19 '09 at 07:16
  • 2
    I think this is yet another hand-me-down from a well-known C API - including the inconsistency between months and days. – Michael Borgwardt Aug 19 '09 at 07:26
  • It actually came from the Taligent project. One of the ONLY things to come out of Taligent. And I don't think they ever completely fixed the day of week bug. – Alice Young Jan 04 '15 at 19:35
27

Just got this one from here, regarding the add and remove methods of List (when parameterized with Integer). For instance:

List<Integer> l = new ArrayList<Integer>();
l.add(20);
l.remove(20); // throws ArrayIndexOutOfBoundsException, because it will try to access index 20
l.remove(new Integer(20)); // this works
Community
  • 1
  • 1
João Silva
  • 89,303
  • 29
  • 152
  • 158
11
String.getBytes()

often the cause of lots of stupid character encoding problems in applications because it uses the underlying platform character encoding.

Jonathan Holloway
  • 62,090
  • 32
  • 125
  • 150
  • 7
    Even more so: FileReader, which doesn't even *have* a constructor that allows you to specify an encoding. – Michael Borgwardt Aug 18 '09 at 06:11
  • 3
    From javadoc: The constructors of this class assume that the default character encoding and the default byte-buffer size are appropriate. To specify these values yourself, construct an InputStreamReader on a FileInputStream. – gnud Aug 18 '09 at 06:18
  • 2
    @gnud: But the point of `FileReader` is to act as a convenience class that allows you to read files without having to use those classes. Indeed, `FileReader` **is an** `InputStreamReader` which uses a `FileInputStream` if you look at the source, hence, it should definitely allow one to set the `Charset` through the ctor, just like its parent class. – João Silva Aug 18 '09 at 06:28
  • 2
    There are a hundred places where Java allows or encourages you to omit the charset. This is unfortunate, as it almost always causes an obscure little bug your US-based testers won't notice. The platform ‘default encoding’ is worse than useless. Is there a Java lint anywhere that will flag this usage? – bobince Aug 18 '09 at 11:04
  • So whats the correct way of getting the `String.getBytes()`? – Shervin Asgari Jun 29 '10 at 11:30
  • 2
    Pass it the charset instead as a String: String.getBytes("UTF8") or by using a Charset object. – Jonathan Holloway Jun 29 '10 at 16:17
10

Just found out about the methods isInterrupted and interrupted of class Thread. From javadoc:

static boolean interrupted()
// Tests whether the current thread has been interrupted.
boolean isInterrupted()
// Tests whether this thread has been interrupted.

The problem is that interrupted actually clears the interrupted status besides doing the test, while isInterrupted just tests the status.

João Silva
  • 89,303
  • 29
  • 152
  • 158
  • 1
    This is indeed very confusing. Be careful when using static interrupted() method. On the other hand, it's handy when doing `if (Thread.interrupted()) throw new InterruptedException();` (You usually want to clear interrupted status when throwing this exception). – Peter Štibraný Aug 19 '09 at 07:39
8

It's probably not the worst method, but I never liked this one:

Suppose x is a list known to contain only strings. The following code can be used to dump the list into a newly allocated array of String:

String[] y = x.toArray(new String[0]);

Passing an String array of the size 0 to the method just seems crazy and unintuitive to me.

Otto Allmendinger
  • 27,448
  • 7
  • 68
  • 79
  • 4
    Yes, it's a total bummer, but it's the only way to infer the type, due to type erasure. – João Silva Aug 18 '09 at 15:43
  • 6
    Actually you get slightly better performance if you write x.toArray(new String[x.size()]). Then it fills the array you allocated and passed in, rather than looking at the type of the passed-in array, throwing it away, and creating a new array. But I agree, it's very awkward. I would have thought that when they added templates they would have created a cleaner alternatively, but apparently not. – Jay Aug 18 '09 at 16:32
  • @Jay this common wisdom is incorrect. The array you allocate needs to be zero'd, while if you let `toArray` handle it, it can take a short cut, see here: https://stackoverflow.com/questions/174093/toarraynew-myclass0-or-toarraynew-myclassmylist-size – john16384 Apr 26 '23 at 10:20
6

InputStream.read(byte[])

Doesn't fill the array; instead it reads an arbitrary number of bytes and returns that number. You have to loop. Nasty, because it works correctly for small arrays most of the time. I don't think anyone gets this right the first time they use it.

Michael Borgwardt
  • 342,105
  • 78
  • 482
  • 720
  • @Micheal: but it has to work that way! Imagine the howls from the J2ME crowd if the (only) block read method allocated its own byte array. – Stephen C Aug 18 '09 at 06:28
  • It wouldn't have to; Ideally the looping would be implemented in the method. Since it blocks anyway, there is no conceivable advantage to the way it works now. – Michael Borgwardt Aug 18 '09 at 07:28
  • 13
    It works exactly the way a C programmer would expect it to ;P – KitsuneYMG Aug 18 '09 at 10:05
  • 1
    Yep. I'm with @kts too. I got it right the first time because I got it wrong the first time I used it in C :) – Dave Ray Aug 19 '09 at 00:59
  • 1
    @Michael: what about the last read? How can read return a full buffer if you reached EOF when the buffer is half full??? – Stephen C Aug 19 '09 at 07:26
  • Yes, you'd still have to return the number of bytes read, but the caller wouldn't have to do the loop, and in many cases (mainly when reading files) you can preallocate the buffer to the correct size. – Michael Borgwardt Aug 19 '09 at 08:25
6

Some redditor noticed that String.substring leads to memory leaks, because internally it does not copy substring, but just copies pointer to whole string + offset + length. So if you expected whole string to be collected by GC, you are screwed.

http://www.reddit.com/r/programming/comments/8ydvg/the_dangers_of_stringsubstring/c0au0gj

maykeye
  • 1,286
  • 1
  • 12
  • 16
  • 1
    Well, because strings are immutable this makes sense as it allows for sharing. Of-course there is the subject of having a small substring from a large string in which case the string is not GCed. – Shimi Bandiel Aug 19 '09 at 08:32
  • 1
    Ohhh yes... I've run into that... getting a list of words from a few hundred 100k-2mb files. Took far too long to figure out that OutOfMemoryException. – CoderTao Aug 22 '09 at 01:16
5

my issue is with String's substring method; every time I use it I have to write out the word "hamburger" and "hamburger".substring(4,8) = "urge" to remember how to use it correctly

Jill
  • 539
  • 1
  • 5
  • 12
  • 2
    Most substring/slicing/subset APIs work in a similar manner, so I'm not sure how confusing this really it. I've come to expect that such ranges are clopen—closed on the bottom but open on the top. It makes iteration that much easier, too. – jasonmp85 Jun 03 '10 at 10:07
  • The main reason this is done with ranges is that the width of the (integer) range [4,8] is 5; the range starting at 4 with width 4 is [4,7]. Using floating-point numbers, this is no longer true! This is because the integer range [4,8] should really be written [4,9) (which also makes calculating the widths much easier...) – BlueRaja - Danny Pflughoeft Aug 05 '10 at 17:57
5

Well, System.setOut() will set the value to a final member of System!!!!

Shimi Bandiel
  • 5,773
  • 3
  • 40
  • 49
  • 2
    IMO, the real issue is that System.{in,out,err} should be static methods not static final members. – Stephen C Aug 19 '09 at 07:31
  • wow! noticed that. how is it even possible to set a final variable after initialization? the setter calls another setter which is marked as native. what's going on? – asgs Nov 10 '16 at 20:26
  • Actually, System.{in,out,err} are defined in the JLS as final values which should not be considered as such regarding thread-safety. Nothing like exceptional cases in spec :( – Shimi Bandiel Nov 16 '16 at 10:48
5

I never really understood why the JDBC API consistently starts counting with 1, while the rest of the Java ( and C, C++, C#, ...) universe starts at 0. This applies for column numbers, parameter numbers in prepared statements etc.

mkoeller
  • 4,469
  • 23
  • 30
  • 1
    In itself, I think the designer of the JDBC API made the right decision starting numbering at 1. But in light of the rest of the Java universe, it is damn confusing. – Steve McLeod Aug 19 '09 at 07:35
  • i just started working with jdbc and noticed this. At least its clearly defined in the javadocs... – luke May 14 '10 at 05:37
  • This is just to maintain consistency with ODBC...but that's just deferring the question to the designers of ODBC! – Drew Hall Mar 17 '12 at 03:57
4

I agree. I've always been uncomfortable with those methods.

I've even found a bug in our code base that was caused by someone using Integer.getInteger() to parse a string, not realizing that it was looking up the property.

Unfortunately, of course, there is no way that the API can ever be removed, for backwards-compatibility reasons.

Avi
  • 19,934
  • 4
  • 57
  • 70
3

I'm not sure if anyone still uses this but the error message from DocumentBuilder.parse() if something goes wrong is (almost?) always "Content is not allowed in prolog." even if the real reason was something else entirely.

Buhb
  • 7,088
  • 3
  • 24
  • 38
3

BigDecimal.setScale(int) a setter that returns a BigDecimal hmmm

keuleJ
  • 3,418
  • 4
  • 30
  • 51
  • 2
    This is a common pattern...and there's a proposal to make all setters act this way. It allows method chaining. Person person = new Person().setFirstName("James").setLastName("Gosling"); – Steve McLeod Aug 19 '09 at 07:34
  • 1
    BigDecimal is *immutable*, so it can't set scale on existing object, but has to return new one. – Peter Štibraný Aug 19 '09 at 07:41
  • 2
    @Peter: I know that, but I think the name of the method is a bit misleading. In my intuition a setter changes something on an object. So maybe I would call the method BigDecimal.convertToScale(int) or something like that. But hey, it's clearly documented, so it's not that big of a problem. – keuleJ Aug 19 '09 at 08:12
  • 1
    I see. I agree that it is little misleading. – Peter Štibraný Aug 19 '09 at 10:40
3

One of my pet peeves with Java is the fact that a failure in Integer.parseInt("SomeString") merely states that there was a parse error, failing to tell us what the "SomeString" was. Because of this, it is sometimes necessary to do loads of debugging to find out what the string was. If the error mesage included the erroneous string, tracking down the problem would be much quicker.

belugabob
  • 4,302
  • 22
  • 22
2

I wouldn't file it under "Most Awkward", but java.security.MessageDigest.getInstance() gave me some confusion.

I'm typically used to see a "getInstance()" method return a Singleton.

If a method is to return a New Instance, i might expect to see a MessageDigestFactory.newInstance(), or at the very least a newInstance() on the MessageDigest class instead of their getInstance() method.

See: MessageDigest.getInstance()

From what I've tested, MessageDigest.getInstance() returns a New Instance, every single time it's invoked.

Chris Holland
  • 9
  • 3
1

java.util.Date.getDate() returned a number, 1-31. getDayOfMonth() is the accurate way to explain it, whereas you were usually trying to remember getTime().

Cannot wait for Joda Time to take over.

Dean J
  • 39,360
  • 16
  • 67
  • 93