avoid if else using gauva how to

726 views
Skip to first unread message

fachhoch

unread,
Apr 20, 2011, 1:19:21 PM4/20/11
to guava-discuss
I dont like writing if else blocks, I always try to avoid it using
apache commons chain ,
I am wonder if guava has anything simpler than common chain , the
problem with commons chain is I have to put that code in try catch
block as it throws checked exception , and I would like to know if
gauva has anything better
I looked at functions ,ordering etc but could not figure out ,
here i have the code ,please educate me how to write this in gauva in
much elegant way


if(eaProgramFinding.getStgFinding().getFindingSource().equals(FindingSource.APP))
{
if(multiFindingProgramDTO!=null){
return !
eaProgramFinding.getStgFinding().equals(multiFindingProgramDTO.getMutiProgramFinding());
}else{
return true;
}
}else {
return false;
}
also I created a paste bin for my code just to read code

here is the link

http://pastebin.com/kPRnnP2T

Kevin Bourrillion

unread,
Apr 20, 2011, 1:29:41 PM4/20/11
to fachhoch, guava-discuss
On Wed, Apr 20, 2011 at 10:19 AM, fachhoch <fach...@gmail.com> wrote:
I dont like writing if else blocks,

Translation: you don't like Java.  Try a language that doesn't use if/else blocks?  They are as natural to Java as 'public class Foo'.


--
Kevin Bourrillion @ Google
http://guava-libraries.googlecode.com

fachhoch

unread,
Apr 20, 2011, 1:36:46 PM4/20/11
to guava-discuss
what I mean I try to avoid if possible like using commons chain my
code would be



try{
return new ChainBase(){
{
addCommand(new Command() {
@Override
public boolean execute(Context context) throws Exception {
return
eaProgramFinding.getStgFinding().getFindingSource().equals(FindingSource.APP) ?
new Supplier<Boolean>() {
@Override
public Boolean get() {
return multiFindingProgramDTO==null ? true :new
Supplier<Boolean>() {
@Override
public Boolean get() {
return !
eaProgramFinding.getStgFinding().equals(multiFindingProgramDTO.getMutiProgramFinding());
}
}.get();
}
}.get() :false;
}
});
}


}.execute(new ContextBase());
}catch (Exception e) {
throw new RuntimeException(e);
}


I am wondering using gauva I can make the code more readable ?

On Apr 20, 1:29 pm, Kevin Bourrillion <kev...@google.com> wrote:

Willi Schönborn

unread,
Apr 20, 2011, 1:42:05 PM4/20/11
to guava-discuss
Honestly, how is this code more readable than simple if-else-statements?

Greetings,
Willi


fachhoch

unread,
Apr 20, 2011, 1:43:37 PM4/20/11
to guava-discuss
here one more better example

try{
new ChainBase(){
{
addCommand(new Command() {

@Override
public boolean execute(Context context) throws Exception {
return multiFindingProgramDTO.getMutiProgramFinding()==null ?new
Functor<Boolean>(){
public Boolean call() {
validators.add(noFinalOCD);
return true;
}
}.call() : false;
}
});
addCommand(new Command() {

@Override
public boolean execute(Context context) throws Exception {
return !
eaAuditProgram.equals(multiFindingProgramDTO.getLeadProgram())&&!
multiFindingProgramDTO.getNonLeadPrograms().contains(eaAuditProgram)?
new Functor<Boolean>() {
public Boolean call() {
validators.add(noFinalOCD);
return true;
};
}.call() :false;

}
});
addCommand(new Command() {

@Override
public boolean execute(Context context) throws Exception {
return !
eaAuditProgram.equals(multiFindingProgramDTO.getLeadProgram())&&multiFindingProgramDTO.getNonLeadPrograms().contains(eaAuditProgram) ;

}
});

addCommand(new Command() {

@Override
public boolean execute(Context context) throws Exception {
validators.add(nonLeadProgramsResolution);
validators.add(noFinalOCD);
return true;
}
});
}


}.execute(new ContextBase());
}catch (Exception e) {
throw new RuntimeException(e);
}




On Apr 20, 1:29 pm, Kevin Bourrillion <kev...@google.com> wrote:

fachhoch

unread,
Apr 20, 2011, 1:57:39 PM4/20/11
to guava-discuss
there are several design patterns which help write code to avoid if
else , I want to write better code, but for simple conditional logic
I try to use commons chain or chain of responsibility pattern , I
think the code above looks bad becasue I have several nested inner
classes , If I had created these classes as non nested and just call
the new ...() then I think it would not be that bad.

On Apr 20, 1:42 pm, Willi Schönborn <w.schoenb...@googlemail.com>
wrote:
> Honestly, how is this code more readable than simple if-else-statements?
>
> Greetings,
> Willi
>

Dimitris Andreou

unread,
Apr 20, 2011, 2:38:42 PM4/20/11
to fachhoch, guava-discuss
Wait, people still reading design pattern books? Isn't that a bit nineties?

My 2c, if you want to write better code, you can start by reading guava code. Good code != code that best fits documented patterns

Robert Konigsberg

unread,
Apr 20, 2011, 1:52:50 PM4/20/11
to fachhoch, guava-discuss

Brian Duff

unread,
Apr 20, 2011, 4:10:34 PM4/20/11
to fachhoch, guava-discuss
The code you included is significantly less readable than it would be if implemented using simple conditional logic.

What is the actual reason you don't like / want to avoid if/else?

Thanks,
Brian

On Wed, Apr 20, 2011 at 10:57 AM, fachhoch <fach...@gmail.com> wrote:

Johan Van den Neste

unread,
Apr 20, 2011, 4:38:48 PM4/20/11
to Robert Konigsberg, fachhoch, guava-discuss

The functional style of this example has at least 1 benefit: it's lazy.
If you don't need the laziness, then yes, the imperative style is
often easier on the eyes.

I got used to Function though. I don't see the plumbing anymore, and
if you name your Function instances nicely, then something like
'Iterables.transform(listOfX, toY)' looks alright. You can reduce the
damage to your teammates' eyes by trying to isolate the Function
plumbing a bit.

You can get a win if you can compose functional operations though.
Writing them out imperatively quickly isn't so readably anymore
either.

fachhoch

unread,
Apr 20, 2011, 4:53:48 PM4/20/11
to guava-discuss
what I meant is to avoid if else for complex logic , I don't like
putting 10 if else block or writing entire logic using if else
blocks, I would like to use some design pattern , so I started using
chain of resposibility pattern, I use commons chainbase but one
problem with that is I it throws checked exception and I have to
catch for every use , but still i can extend that and do my own
stuff .
but some times even chain of responsibility does not help me for cases
where I needed nested if else I have to go with nested chain in
that case it makes it more complex , but I can create these objects
seperately and just call them reuse them etc. so my question is
there anything like this code below or something more useful etc

public static <T> void evaluate(Supplier<Boolean> condition,
Supplier<T> performer ){
if(condition.get()){
performer.get();
}
}







On Apr 20, 4:10 pm, Brian Duff <bd...@google.com> wrote:
> The code you included is significantly less readable than it would be if
> implemented using simple conditional logic.
>
> What is the actual reason you don't like / want to avoid if/else?
>
> Thanks,
> Brian
>
> > > > To get help:http://stackoverflow.com/questions/ask(usethe tag

Colin Decker

unread,
Apr 20, 2011, 5:02:58 PM4/20/11
to guava-discuss
Never, ever use a design pattern just because you think you should be using a design pattern. Your attempts to force design patterns on something that should be very simple are making your code anything but simple. Guava doesn't provide anything that's going to make it easier for you to do what you want because you really shouldn't be doing it.

-- 
Colin


Craig Berry

unread,
Apr 20, 2011, 5:13:56 PM4/20/11
to Colin Decker, guava-discuss
Agreed. And yet, I also agree that multi-branch if-else statements
(and similarly, case statements) are often a "bad code smell"
indicating that a method or class "knows" too much about its
collaborators. Design patterns can be useful in correcting such
knowledge leaks.

On Wed, Apr 20, 2011 at 2:02 PM, Colin Decker <cgde...@gmail.com> wrote:
> Never, ever use a design pattern just because you think you should be using
> a design pattern. Your attempts to force design patterns on something that
> should be very simple are making your code anything but simple. Guava
> doesn't provide anything that's going to make it easier for you to do what
> you want because you really shouldn't be doing it.

--
Craig Berry
Software Engineer
Google (Santa Monica CA)

Chris Povirk

unread,
Apr 20, 2011, 5:27:10 PM4/20/11
to Craig Berry, fachhoch, guava-discuss
On Wed, Apr 20, 2011 at 5:13 PM, Craig Berry <cbe...@google.com> wrote:
> Agreed. And yet, I also agree that multi-branch if-else statements
> (and similarly, case statements) are often a "bad code smell"
> indicating that a method or class "knows" too much about its
> collaborators. Design patterns can be useful in correcting such
> knowledge leaks.

+1. If I recall correctly, the book _Refactoring_ describes a
"Replace Conditional with Polymorphism" refactoring, which is
frequently invaluable. But not all |if| statements can be similarly
eliminated. In particular, if you're explicitly passing in two
continuations, one for the |true| case and one for the |false| case,
the code you're writing is the same kind of Master Control Flow as an
|if| statement, and you aren't getting the benefits of polymorphism.

Here's a similar answer to a similar question:
http://stackoverflow.com/questions/519515/how-can-polymorphism-replace-an-if-else-statement-inside-of-a-loop/519537#519537

Jed Wesley-Smith

unread,
Apr 22, 2011, 3:27:17 AM4/22/11
to Johan Van den Neste, Robert Konigsberg, fachhoch, guava-discuss
it is amusing that people still want to explicitly create intermediate ephemeral Lists for these kind of trivial transformations rather than use the functional approach. The ONLY argument against it being that java has the clumsiest possible syntax for an inline anonymous function, but if you separate out the function definition (which allows you to share and compose the logic) you actually get a net reduction in LOC and better intentional readability:

return Lists.transform(strings, toInt);

This is way better than:

ImmutableList.Builder<String> builder = ImmutableList.builder();
for (String s : strings)
  builder.add(Integer.valueOf(s));
return builder.build();

the fact that it can be a lazy transform also means that there aren't intermediate arrays that the data is copied through which can be a significant performance profile improvement.
cheers,
- jed.

Chris Povirk

unread,
Apr 22, 2011, 9:09:57 AM4/22/11
to Jed Wesley-Smith, Johan Van den Neste, Robert Konigsberg, fachhoch, guava-discuss
On Fri, Apr 22, 2011 at 3:27 AM, Jed Wesley-Smith
<jed.wes...@gmail.com> wrote:
> it is amusing that people still want to explicitly create intermediate
> ephemeral Lists for these kind of trivial transformations rather than use
> the functional approach. The ONLY argument against it being that java has
> the clumsiest possible syntax for an inline anonymous function

Yes and no. It's the most common reason, and, as you point out, if
you can easily extract a Function object into a field or another
method, transform() can be shorter and more intention-revealing than a
loop. This is especially true if you have a Function that can be
reused for multiple calls -- a transform() here, an
ordering.onResultOf() there, a Multimaps.index() over there.... Of
course, opinions on when to use transform() still vary widely. I tend
to think that, if the Function perform an obvious, simple, easily
nameable job, and if you're willing to give it its own method or
field, the transform() approach will often win out. (When we're
working on collections code, sometimes a view is *required*, so
transform() is the only game in town.) Still, while "the clumsiest
possible syntax" is "just" syntax, but we've all seen code (heck, I've
*written* code) that overuses functional programming enough that the
syntax gets in the way. IMO, this arises mostly in unusual cases.
One example is nested functional programming calls, where you see
something like "Predicates.filter(Iterables.transform(inputs,
nameFunction), Predicates.and(before(x), after(y))." Here static
imports can help, but "filter(transform(inputs, nameFunction,
and(before(x), after(y))" is still messy. We could further you out by
providing filter() and transform() as infix methods on an object, and
we'll probably eventually do this, but in the meantime, syntax is a
barrier for nested calls. I've written similarly ugly code by trying
to operate functionally on nested data structures. And of course, if
your transformation is anything that throws checked exceptions, you
have to do it by hand. In the end, we all have to use our judgment on
each instance, and, while I don't want to put words in Rob's mouth, if
he were forced to choose between the Chain and transform() examples in
this thread to put into his code, I suspect he'd pick transform(). We
can disagree over where the line is between imperative and functional
programming, but I don't think anyone is trying to equate those two
examples.

All that said, I do want to emphasize that there are additional
arguments against the *lazy* functional approach:

1) If you function is expensive, you may prefer to perform it only
once. (If you do nothing but loop over the result of transform() a
single time, then this is already happening, but it's easy to forget
if you pass the List far from the point at which it was created.)

2) If your function creates mutable objects, you probably want to
create each output object only once. My smug response to this is
"Well, of *course* I wouldn't use mutable objects -- here or anywhere
else that they'd be a problem." But a Googler had a fascinating bug a
while back because of this. Since I'm bringing it up in this context,
the problem may be obvious, but it wasn't at the time, when we had no
idea whether Futures.successfulAsList (coming soon to a Guava near
you!), his CheckedFuture implementation, or what was wrong. Here's
his code. Somehow, even though successfulAsList(...).get(...) was
returning a list with entries, doSomething was never being called
because checkedGet kept timing out.

List<CheckedFuture<String, MyException>> futures = ...;
try {
Futures.successfulAsList(futures).get(TIMEOUT, TimeUnit.SECONDS);
}
...
for (CheckedFuture<String, MyException> future : futures) {
try {
doSomething(future.checkedGet(0, SECONDS));
} catch (MyException e) {}
}

The problem was hidden in that first "...." He was creating the list
with Lists.transform(). I don't think of Futures as being "mutable"
in the "mutable objects are evil" sense because they mostly hide their
state, but here that matters.

You still might legitimately claim that you wouldn't make this
mistake, and that's fine, but I want to counter the idea that lazy
transformations are a safe default. The decision to go with either a
for() loop or a transform() call requires some consideration

3) If the transformation may fail due to programmer errors (==
RuntimeExceptions, e.g., a NPE if any element is null), you may prefer
to perform all transformations immediately so that any failure occurs
as soon as possible when the most information about the errant caller
is available.


So again, I'm more of a fan of transform() than Rob is, and I'm in
agreement with you on your example. Still, I think that reasonable
people can disagree on your example and agree on extreme cases like
Chain. And regardless of what our usual preferences are, we should
consider each instance on a case-by-case basis.

Chris Povirk

unread,
Apr 22, 2011, 9:51:33 AM4/22/11
to Jed Wesley-Smith, Johan Van den Neste, Robert Konigsberg, fachhoch, guava-discuss
One more edge case:

4) If your transformer is a Function<SomeVeryLargeObject, Integer>
(say, veryLargeObject.getId()), then the view returned by transform()
will retain references to the full objects, keeping them alive even
though you may need only their IDs.

I also meant to explicitly point out that not only could you avoid
these problems by calling ImmutableList.copyOf(transform(...)), but
we've been considering a Lists.transformedCopy() to do it for you.
We're still not sure whether to add it or not.

Jed Wesley-Smith

unread,
Apr 23, 2011, 12:59:06 AM4/23/11
to Chris Povirk, Johan Van den Neste, Robert Konigsberg, fachhoch, guava-discuss
On 22 April 2011 23:09, Chris Povirk <cpo...@google.com> wrote:
On Fri, Apr 22, 2011 at 3:27 AM, Jed Wesley-Smith
<jed.wes...@gmail.com> wrote:
> it is amusing that people still want to explicitly create intermediate
> ephemeral Lists for these kind of trivial transformations rather than use
> the functional approach. The ONLY argument against it being that java has
> the clumsiest possible syntax for an inline anonymous function

Still, while "the clumsiest
possible syntax" is "just" syntax, but we've all seen code (heck, I've
*written* code) that overuses functional programming enough that the
syntax gets in the way.

The only problem I have with that statement is the word over-uses. There are plenty who will argue that FP has no place in Java and I am not interested – personally I find idiomatic Java difficult to read (in the sense that there is just so much of it). At least with a functional approach I can generally guarantee the runtime invariants.
 
All that said, I do want to emphasize that there are additional
arguments against the *lazy* functional approach:

We have just been talking about the divide between the "lazy default is the root of all evil" and "lazy is the best eva!" camps in the FP world. Interestingly, a lot of Haskell's library was a result of the laziness choice and the pure discipline it then forced them into. I personally don't have a dogmatic opinion on the subject but take it case by case.

1) If you function is expensive, you may prefer to perform it only
once.  (If you do nothing but loop over the result of transform() a
single time, then this is already happening, but it's easy to forget
if you pass the List far from the point at which it was created.)

You can be lazy and still memoize. Feature Request!
Well, I think it is important that the programmer understand the runtime semantics of what they using. One of the hairier bits of code I have is an asynchronous completion service wrapper that completely abstracts away all the complexity of submitting jobs and the like. There is a need there to know which bits to copy and what to perform lazily, but it wasn't _that_ difficult.


The specifics of that example are interesting though. In particular the new Futures.successfulAsList – the fact that it isn't assigned to anything is interesting, how is it meant to work? I would have assumed that you are still dealing with the original list of futures where some may have already failed.

3) If the transformation may fail due to programmer errors (==
RuntimeExceptions, e.g., a NPE if any element is null), you may prefer
to perform all transformations immediately so that any failure occurs
as soon as possible when the most information about the errant caller
is available.
 
Well, there's not a lot you can do to stop people shooting themselves in the foot. 

So again, I'm more of a fan of transform() than Rob is, and I'm in
agreement with you on your example.  Still, I think that reasonable
people can disagree on your example and agree on extreme cases like
Chain.  And regardless of what our usual preferences are, we should
consider each instance on a case-by-case basis.

Yep agree you need to think. I will add that for me I haven't yet seen an old school transform case that isn't better served with the filter/transform functional style. For loops are for side-effectful (A => Void) programming.
--
cheers,
- jed.

Chris Povirk

unread,
Apr 26, 2011, 12:37:35 PM4/26/11
to Jed Wesley-Smith, Johan Van den Neste, Robert Konigsberg, fachhoch, guava-discuss
>> All that said, I do want to emphasize that there are additional
>> arguments against the *lazy* functional approach:
>
> We have just been talking about the divide between the "lazy default is the
> root of all evil" and "lazy is the best eva!" camps in the FP world.
> Interestingly, a lot of Haskell's library was a result of the laziness
> choice and the pure discipline it then forced them into. I personally don't
> have a dogmatic opinion on the subject but take it case by case.

I can wave my hands a bit about functional programming vs. imperative,
but I'm useless once we get into "second-order" considerations like
these. I'm glad that we have people around with that experience.

>> 1) If you function is expensive, you may prefer to perform it only
>> once.  (If you do nothing but loop over the result of transform() a
>> single time, then this is already happening, but it's easy to forget
>> if you pass the List far from the point at which it was created.)
>
> You can be lazy and still memoize. Feature Request!

We do in fact have some work underway to have MapMaker do this for
you. Of course, you can build it yourself on top of MapMaker today,
and I'll paste in some of our internal code for anyone who just can't
wait:

public static <F, T> Function<F, T> memoize(
Function<? super F, ? extends T> delegate, MapMaker mapMaker) {
final Map<F, T> map = mapMaker.makeComputingMap(delegate);
return new Function<F, T>() {
@Override
public T apply(F from) {
return map.get(from);
}
};
}

Chris Povirk

unread,
Apr 27, 2011, 7:33:56 PM4/27/11
to Jed Wesley-Smith, Johan Van den Neste, Robert Konigsberg, fachhoch, guava-discuss
> The specifics of that example are interesting though. In particular the new
> Futures.successfulAsList – the fact that it isn't assigned to anything is
> interesting, how is it meant to work? I would have assumed that you are
> still dealing with the original list of futures where some may have already
> failed.

Sorry, I meant to respond to this but skipped over it while writing.

It's possible that, in simplifying the code from the original example,
I've messed things up, and it's possible that I misinterpreted the
author's intent, but I believe the goal is: "When all tasks are done,
or when |SECONDS| seconds have elapsed, process all available
results." One way to do this is to maintain a deadline to use for
each checkedGet call:

List<CheckedFuture<String, MyException>> futures = ...;

long deadline = nanoTime() + SECONDS.toNanos(TIMEOUT);


for (CheckedFuture<String, MyException> future : futures) {
try {

doSomething(future.checkedGet(deadline - nanoTime(), NANOSECONDS));
} catch (MyException e) { ... }
}

That's actually shorter than the code I gave:

List<CheckedFuture<String, MyException>> futures = ...;
try {
Futures.successfulAsList(futures).get(TIMEOUT, TimeUnit.SECONDS);

} catch (Exception e) {} // deal with it later


for (CheckedFuture<String, MyException> future : futures) {
try {
doSomething(future.checkedGet(0, SECONDS));

} catch (MyException e) { ... }
}

And that's not to mention that the deadline approach is more likely to
be able to parallelize its doSomething() calls with the work of the
futures or that we really ought to deal with InterruptedException
specially instead of using a blanket "catch (Exception e)." So I'm
not sure what the advantage of the successfulAsList() approach is.
Some people don't realize that it's safe to pass a negative deadline
to get()/checkedGet(), so perhaps he thought he would need an if()
statement there? I can follow up internally if you'd like, but I
think I've at least got the motivation right, if not the fine details
of his approach.

Reply all
Reply to author
Forward
0 new messages