Lord Of Texas posted:

Here's a design question. It has members of my development team on opposite sides, so I thought I'd get the opinions of the thread.
-----

Say you're calling a method that returns some business resource from an external service.

code:

public ThingBean getThingBean(params){}

The requirements state that if the requested resource is not found, the application should recover (i.e. throwing an unchecked exception is out).

Your options when the resource is not found are:

1) return null
2) throw a checked exception for the client code to handle
3) return an empty ThingBean with no fields populated

Aleksei Vasiliev posted:

Option 3 is the solution I'd go with if I was asked to come up with the worst solution.

If throwing an unchecked exception is out, then so is returning null, as they can fail to handle that (and cause a NPE).

quote:

1 is a terrible thing because very few people check for null, especially in Java. This leads to a NullPointerException.
3 is terrible because it requires you to test all of ThingBean
2 is the best choice, although people will often just write try{ } catch (Exception e) rather then the specific exception.

Lord Of Texas posted:

Yeah, this is basically exactly how I feel, glad to know I'm not alone. The principal devs on my project prefer using null as a valid response and putting the onus on the client to check for null.

Since clients can just catch(Exception){do nothing} at some point you do have to trust the client to not be terrible, but I feel like a checked exception in this case gives the client less leeway to gently caress up.

Doctor w-rw-rw- posted:

Option 4: use Optional from Guava or something like it to indicate an intended absence of a value.

quote:

I'm with your devs. If you throw an Exception, you make the client do more boilerplate try/catch and flow control than simply checking for null or allowing the NPE to happen and be caught.

quote:

There are many cases where you don't want the wrapper method to crash out entirely if an object can't be retrieved. To think about it on another level, do you really want HashMap to throw an exception if it doesn't have a key?

Lord Of Texas posted:

Is this:

code:

try{
Thing thing = getThing();
}catch(MissingThingException e){

}

Really that much more boilerplate than:

code:

Thing thing = getThing();
if (thing==null){

}

quote:

If you are talking about getThing(), let's assume that getThing() doesn't have any reason to continue once it has determined that the requested Thing doesn't exist.

Lord Of Texas posted:

Yeah, I've considered some variation of Null/Missing Object, i.e. returning this guy when Thing isn't found:

code:

MissingThingBean extends ThingBean{

	@Override
	public ThingCode getThingCode(){
		return new ThingCode("The requested Thing was not found");
	}
}

But that means
1) The client is not forced to check for what instance he has
2) if the client doesn't check for the instance, the MissingThingBean gets propagated up to some other layer that may or may not handle it gracefully.

Optional looks like a solid choice though - thanks for the recommendation!

Doctor w-rw-rw- posted:

Another tip - don't feel afraid to *not* check if its value is present if you have a strong reason to expect that it is not going to be null, because that is an exceptional circumstance that should indeed be detected by an exception.

Lord Of Texas posted:

Maybe I'm misunderstanding, but in that case wouldn't you just not use Optional? An Optional return value means the contract is "This can be empty and you should account for that", but if null is not a valid response, that contract says "This cannot be empty, don't worry about accounting for that".

rhag posted:

I personally believe that the contract should be something like this:
1. If the method returns a collection, then it should never return null. Return an empty collection if there are no things there.
2. If the method returns a single object, it returns null if there are no objects that satisfy the condition (find(id) for example).

The best solution is the one provided by Scala with the Optional support, but in Java an implementation of that would only lead to more verbose code for what otherwise should be a simple check.
The worst solution is throwing a runtime exception (like JPA for createQuery(...).getSingleResult()).

Of course, to any solution there are exception cases, and there could indeed be times where returning new Thing("i don't have any") may be valid and preferred, but i personally think this should not be the normal way of doing things.

rhag posted:

I personally believe that the contract should be something like this:
1. If the method returns a collection, then it should never return null. Return an empty collection if there are no things there.
2. If the method returns a single object, it returns null if there are no objects that satisfy the condition (find(id) for example).

The best solution is the one provided by Scala with the Optional support, but in Java an implementation of that would only lead to more verbose code for what otherwise should be a simple check.
The worst solution is throwing a runtime exception (like JPA for createQuery(...).getSingleResult()).

Of course, to any solution there are exception cases, and there could indeed be times where returning new Thing("i don't have any") may be valid and preferred, but i personally think this should not be the normal way of doing things.

rhag posted:

I personally believe that the contract should be something like this:
1. If the method returns a collection, then it should never return null. Return an empty collection if there are no things there.
2. If the method returns a single object, it returns null if there are no objects that satisfy the condition (find(id) for example).

The best solution is the one provided by Scala with the Optional support, but in Java an implementation of that would only lead to more verbose code for what otherwise should be a simple check.
The worst solution is throwing a runtime exception (like JPA for createQuery(...).getSingleResult()).

Win8 Hetro Experie posted:

Instead of Optional, using one of the NotNull annotations and an IDE with an well-integrated static analysis for them would be better if you can make it work in your organization.

I don't like tweaking the return type or wrapping the value. A variable holding the return value of a method has several properties: formal coming from its type, some that are promised by the name of the variable, informal that are inferred from the method's name. Adding more properties by adding annotations scales easily; changing types much less so.

notMordecai posted:

I posted not too long ago but I have another unit testing question. Still sorta new to it and trying to push myself in learning new techniques.

I have this class that I kinda shortend (for this example) that I need to test.

code:

public class HandlerFactory extends SecondHandlerFactory
{
	//To hold singleton of this class.
	private static SecondHandler factoryInstance = null;

	private HandlerFactory() throws HandlerCreationException
	{
                super();
	}

	protected InterfaceExample createSomethingByKey(String key) throws HandlerCreationException
	{
		InterfaceExample myNewHandler = null;
		
		if (StringFunctions.isEqualIgnoreCase(key, "Phone"))
		{
			myNewHandler = new PhoneHandler();
		}
	}
}

I'm having major trouble figuring out to set up this test to even fall into the createSomethingByKey(). Even then, I can't even figure out how I would do my assertEquals() method correctly.

Right now, I have stuff set up to to do a createInstance(); but nothing seem to make any sense to me in how to fall into this. The added variable and interfaces are making me hella confused here. It seems simple to test and is a very, very short class- but not being exposed to this (Java isn't even my first language) is kind of a hard wall for me to get over).

Any help on how to set up either a standard JUnit or Mockito (doesn't matter really) on how to set all this up? Also the fact that's protected is making me even more clueless since proper standard is to have Unit Tests in another package.

rhag posted:

The unit tests should be in the same package as the class they're testing. Another source folder, sure, but the same package. At that point you can make things (members, methods, etc.) package private and call them,assign them, etc.

If you really cannot make that protected method package private, you can do things like subclassing HandlerFactory in your unit test, that can have a method that calls createSomethingByKey . But really, that's a slippery slope, since then you end up testing the subclassed class not the class you want to actually test. But in some cases it may be the only thing you can do.

ManlyWeevil posted:

Is there any significant difference between

code:

try {
...
} catch (Exception e) {
    if (e instanceof SomeException) {
        doSomething(e);
    } else if (e instanceof SomeOtherException) {
        doSomethingElse(e);
    }
}

And the usual

code:

try {
...
} catch (SomeException e) {
    doSomething(e);
} catch (SomeOtherException e) {
    doSomethingElse(e);
}

ManlyWeevil posted:

Is there any significant difference between

code:

try {
...
} catch (Exception e) {
    if (e instanceof SomeException) {
        doSomething(e);
    } else if (e instanceof SomeOtherException) {
        doSomethingElse(e);
    }
}

And the usual

code:

try {
...
} catch (SomeException e) {
    doSomething(e);
} catch (SomeOtherException e) {
    doSomethingElse(e);
}

ManlyWeevil posted:

Doctor w-rw-rw- posted:

The first will catch *all* exceptions silently except the one it checks for. The second, which is more proper, will only catch those two types and their subclasses.

Unless you're wrapping a library you think might be unreliable (and have planned accordingly), it is my opinion that you should NOT use a catch-all for exceptions. If you need something to be run no matter what, like a close() call to a stream or something, then put it in a finally block.

e: beaten to the punch.

ManlyWeevil posted:

Apologies, ignoring the catch all (pretend any unchecked exception is simply rethrown), is there any difference between catch(SomeException e) and if (e instanceof SomeException) ?

ManlyWeevil posted:

code:

try {
...
} catch (Exception e) {
    if (e instanceof SomeException) {
        doSomething(e);
    } else if (e instanceof SomeOtherException) {
        doSomethingElse(e);
    }
}

quote:

JMagick does not attempt to make the ImageMagick API object-oriented. It is merely a thin interface layer into the ImageMagick API.

JMagick currently only implements a subset of ImageMagick APIs.

Joda posted:

code:

		for (int i = 0; i <= k; i++) {	 //Recurse for every value of i up to and including k

...
I must've hosed up the way in which I do the recursion.
...
a number of times to recurse the expression

Nippashish posted:

There's no recursion in your post.

Newf posted:

How much have you played around with this? Try some smaller values for your input 'k' and see what you get. Something like

code:

for (int k=1; k<= 15; k++){
   System.out.println(k + " recursions :  " + cosineTaylor(3.14,k));
}

may be illuminating for you.