3

I'm reformatting some legacy code that I don't fully understand, and in it there are several variable assignments that conditionally assign variables to the output of one of two formatting functions, using exception catching like so:

String myString;
try {
    myString= foo(x);
} catch (Exception e) {
    myString= bar(x);
}

This seems like an abuse of exception handling, and anyway it's a lot of repeated boilerplate code for a lot of variable assignments. Without digging into foo to identify the conditions that might cause exceptions, can I simplify this using a ternary operator expression? I.e. something like this:

String myString = foo(x) ? foo(x) : bar(x)

but catching the exception that might be thrown by foo(x). Is there a way to do this in this one-liner? Or if not, is there a better one-line expression that chooses between two assignments based on a possible exception? I am using Java 8.

workerjoe
  • 2,421
  • 1
  • 26
  • 49
  • 8
    Better keep it as is. Ternary operators don't have that concept. Beside that, `foo(x) ? foo(x) : bar(x)` calls `foo(x)` at least one time too many. – ernest_k Jun 12 '19 at 15:44
  • Complex/long ternary statements can be very difficult to read, and there's no real advantage to making them longer or more complex, so I'd err on the side of readability rather than compactness. – JonK Jun 12 '19 at 15:48
  • I added a follow-up question: if the ternary operator doesn't do it, is there another, analogous kind of Java one-liner that chooses between two assignments based on an exception condition? – workerjoe Jun 12 '19 at 15:52
  • @workerjoe Post your refinements as edits to the body of the Question rather than as Comments. – Basil Bourque Jun 12 '19 at 16:23
  • @BasilBourque I did both. – workerjoe Jun 12 '19 at 16:44

3 Answers3

4

One could use lazy evaluation as follows:

String myString = attempt(() -> foo(x), () -> bar(x));

public static <T> T attempt(Supplier<T> a, Supplier<T> b) {
    try {
        return a.get();
    } catch (Exception e) {
        return b.get();
    } 
}

It is not nice and just would be one stage in the entire refactoring.

One pattern for such constructs you saw would be a result message of a process that maybe throws an IOException.

An ugly pattern would be a NullPointerException or such, when data is optional. Then a complexer redesign with Optional.ofNullable and .map to another Optional might be feasible.

In short I do not see a clean way.

Joop Eggen
  • 107,315
  • 7
  • 83
  • 138
  • 1
    Since `foo` is apparently supposedly capable of throwing `Exception` (not just `RuntimeException`), shouldn't you use a `Callable` instead of a `Supplier`? – Slaw Jun 12 '19 at 18:13
  • @Slaw entirely right. Unfortunately the solution then would no longer be light-weight. The answer now should only handle RuntimeException. Make it an answer. – Joop Eggen Jun 13 '19 at 07:50
  • https://softwareengineering.stackexchange.com/questions/254311/what-is-the-difference-between-callablet-and-java-8s-suppliert for an explanation – Joop Eggen Jun 13 '19 at 08:01
2

Washcloth answer is already clear. Just wanna add a bit though about your statement:

it's a lot of repeated boilerplate code for a lot of variable assignments.

If you don't want to assign a variable repeatedly, you can create a new method to return String value as below:

String myString = assign(x);

String assign(String x) {
    try {
        return foo(x);
    } catch (Exception e) {
        return bar(x);
    }
}

You only need to assign the variable once with the above method.

Bernhard Josephus
  • 695
  • 8
  • 22
1

Considering this case

String myString;
try {
    myString= foo(x);
} catch (Exception e) {
    myString= bar(x);
}

What would happen if foo(x) throws an exception because it can't handle strings with UTF-16 characters, then we would use bar(x) instead.

In your ternary operator case String myString = foo(x) ? foo(x) : bar(x) if you check foo(x) first and it throws an error your entire program would error out which leads us back to putting a try statement around your ternary operator.

Without the original code its hard to say why the developers did it this way but above is an outlined case as to why they chose this design practice. Also good to note that not all legacy code is bad, in this case; the legacy code works, is maintainable, and easy to read for new developers. So as the comment says, best to leave it this way.

Edit

You said you wanted some sort of 1 liner that reduces boiler plate. You could do something like this

void hee(String mystring) {
    try {
        myString= foo(x);
     } catch (Exception e) {
        myString= bar(x);
     }
}

Putting this function in a utility class followed by changing myString = foo(x) to hee(x) would suffice since your original object X is not a primitive java type. This solution is backwards compatible (since this is legacy code I'm unsure what jdk you are using) and requires minimal explanation.

washcloth
  • 2,730
  • 18
  • 29
  • I understand why the ternary operator expression I suggested would fail. My real question, I guess, is whether there's an analogous one-liner that chooses between assignments based on an exception condition, i.e. falling back to a default. – workerjoe Jun 12 '19 at 15:54
  • 1
    @workerjoe above is a simple solution that would work with your existing legacy code – washcloth Jun 12 '19 at 16:10
  • Looks good. Yours was the first answer so I'll mark it correct if nothing radically different comes up in the next day or two. – workerjoe Jun 12 '19 at 16:14
  • You seem to be trying to "return" the result of either `foo` or `bar` by assigning the value to the `myString` parameter. This won't work because Java is _pass-by-value_. – Slaw Jun 12 '19 at 18:17
  • @Slaw "In java the Objects are passed by reference But internally it is pass by value. primitive data is directly pass by value." and "In Java, String is a non primitive data type as it is always written in uppercase. And any data type which starts with upper case is a non-primitive data type." – washcloth Jun 12 '19 at 18:29
  • The value that's passed is the reference to the object. In other words, the reference is copied but it points to the same object. However, reassigning the parameter has no effect on the code that called the method. See https://stackoverflow.com/questions/40480/is-java-pass-by-reference-or-pass-by-value – Slaw Jun 12 '19 at 18:31
  • @Slaw is right, the code for `hee` doesn't work. Moreover, you'd need to redefine it to take `x` as an argument. I think Bernhard's `assign` method is a better, simpler design. Joop's `attempt` is clever, too. – workerjoe Jun 13 '19 at 14:23