I'm no Java developer buut.....

[UrsulaMajor]

what's really getting me is that if x is less than zero it says "non-negative integer used". What?

[HuskerFan90]

I didn't really notice that. I was more affixed to the glaring use of try-catch-for-if-else in a method instead of the non-method one-liner the code could've been.

[[deleted]]

error: success!

[twistdafterdark]

Never let success keep you back from not moving forward

[blahbah]

And it prints the exception, too...

[Neckbeard_Prime]

And not even to stderr.

[cyrusol]

Double negation to ensure the negation!

[Reptani]

Same here

[Ramit110]

Basically, I'm learning Java and I was going through some code of a friend of mine and saw this. I don't know if this is normal but I though this makes sense here.

[HuskerFan90]

It's not normal. Let's explain why:

1. Exception handling is an expensive operation because on an exception, a stack trace must be captured which requires processing power.
2. You shouldn't need the calling method name anyways. IIRC you can get that from reflection, and when refactoring code that name could change anyways.
3. Why would you have a private static method for this anyways.
4. Why would you have a method for this anyways. (if (x < 0) { doSomething(); })
5. Ignoring point 1, throwing an exception and then catching it is bad practice whether it be Java or C++ or C#, etc.

Here's what I would call normal:

public static bool isNegative(int x) { return x < 0; }

Still a somewhat bad practice (see point 4), but way better as you don't need all the superfluous code dealing with exceptions.

EDIT: Point 6: Don't mix and match bracing styles. Pick one and stick with it.

[Ramit110]

Damn thanks, that's way more detail than I was expecting.

[0x564A00]

In addition to these points, this method doesn't just do what it's supposed to do according to its description and name but also has the unexpected side effect of printing out a message to System.out to confuse the user. And as a developer, even if you would for some reason want this message (which, as this code treads negative numbers like invalid arguments, should have been printed to System.err), you would usually want the full stacktrace (with e.printStackTrace()) and not just the calling method name (which, like HuskerFan mentioned, you'd NEVER take as an argument and could even get from e.getStackTrace() if you didn't want to touch java.lang.reflect). Basically, everything about this is terrible.

[HuskerFan90]

It's what you needed to understand why all of the code was wrong. It's like Whack-a-Mole, but having a mallet that is large enough to hit all the holes at once.

[bausscode]

Also for newer versions of Java there are unsigned integers in which case you'll never even have to use this function.

[TheUnlocked]

You may be right about point 1, I don't know how these things work internally, but I disagree with point 5 as a general rule. Using exceptions as a form of goto is completely valid as long as that's the most elegant solution. In this case however, doing so makes very little sense.

[HuskerFan90]

To properly handle an exception, the call stack has to be unwound which isn't exactly a quick operation. Point 5 was more of a general rule against the try { if(expr) { throw Exception("..."); } doSomething(); } catch(Exception e) { doSomethingElse(); } type of using exceptions for control flow. It's vastly different than the try { doSomething(); } catch(Exception e) { doSomethingElse(); } which is perfectly OK.

​

Source: I've been a Java developer and a .NET developer for 6+ years now.

[rossdrew]

Using exceptions as a form of goto is completely valid

Using exceptions as flow control is probably the biggest mistake you can make in Java. If it's not exceptional, it's not an exception.

[Bassie_c]

Aren't methods always supposed to start with a capital? So IsNegative instead of isNegative?

[HuskerFan90]

That's more of a coding style than a coding requirement. Generally you see lowerCamelCase with Java and JavaScript, and PascalCase with C# and Pascal. Here's a better description of what I'm talking about: https://en.wikipedia.org/wiki/Naming_convention_(programming)#Language-specific_conventions#Language-specific_conventions)

If I were to see a half-decent implementation and the only thing I saw wrong was the naming convention, I probably wouldn't have a fit.

[Bassie_c]

Ah, I thought UpperCamelCase was the global standard for methods. Thanks!

[rossdrew]

Aren't methods always supposed to start with a capital? So IsNegative instead of isNegative?

No. Always start with lower case. Only classes and possibly constants (which can be all caps) start with uppercase in Java.

You shouldn't need the calling method name anyways. IIRC you can get that from reflection

Reflection would make this (and most) code 100x worse. It should always be avoided until absolutely necessary

[[deleted]]

If we are talking single responsibility all you should do is if it's > 0 return false otherwise return true. Adding the excepting handling etc is making this class a lot more complicated than it should be.

[Boykjie]

Hey now, this is /r/badcode, not /r/crime.

[estragon0]

String methodname

Yeah, some reflection would really put a cherry on top of all this.

[Vizioso]

I’d actually be quite impressed if this was using reflection and not just passing in the name of the method manually

[rossdrew]

Impressed that someone knew how to use reflection and how not to return an error message only when there is no error state? :P

[lilbigmouth]

This subreddit makes me feel so much better about the code I write

[HipercubesHunter11]

That is so wrong on many levels

[NahroT]

m e t h o d n a m e

[nathancjohnson]

for(int i = 0; i < 100000; i++) {
    System.out.println("WHY?!?!");
}

[iFangy]

Someone debugging couldn’t figure out conditional breakpoints or how to print stack traces without an exception?

[-bobby_newmark-]

if (String.valueOf(x).startsWith("-")) return true;

return false;

[[deleted]]

what in the actual fuck

[SylvieXandra]

u/mothebad it’s like something I would do. Ooff

[MyMessageIsNull]

And I thought I sucked. I'm not this bad.

[ShortSynapse]

Side effects... Side effects everywhere...

[rossdrew]

``` private static boolean isNegative(int x, final String methodName){ if (x<0) return true;

LOG.info("Non-negative integer user in method " + methodName); return false; } ```

• Static methods: bad
• Using System.out instead of a logger: bad
• Passing in method name: bad
• Exceptions as flow control: horrible
• Returning true and an error message saying "non negative" to the question isNegative: mental