1

I have the following code and it works using the good old java:

List<Bar> repo = ArrayList<>(); public Bar foo(int id) { for(Bar c: repo){ if(c.getId() == id) return c; } Bar target = new Bar(); target.setFooo(""); target.setId(0); return target; }

However, I was trying to make it a little better, (i.e. just trying to learn lambdas) 

public Bar foo(int id) { Bar target = repo.stream().filter(c -> c.getId() == id) .findFirst().orElse(null); if(target == null){ target = new Bar(); target.setFooo(""); target.setId(0); } return target; }

But the code above returns an ArrayOutOfBounds Exception and I am not really sure how (since it is a list) or why.

Improve this question
4
  • 1
    As far as I can tell, your two versions are equivalent. We need to know more details to tell what is causing the exception. Commented Nov 6, 2015 at 1:48
  • The code above works for me. You likely have an error elsewhere. Commented Nov 6, 2015 at 1:52
  • 6
    It would be better to drop the .orElse(null), change to Optional<Bar> target, and use if (bar.isPresent()) return bar.get();, otherwise construct a Bar newTarget. --- Even better, use .orElseGet(() -> { Bar newTarget = new Bar(); newTarget.setFooo(""); newTarget.setId(0); return newTarget; }) Commented Nov 6, 2015 at 1:52
  • 2
    the 1st version looks better to me :) Commented Nov 6, 2015 at 2:30

2 Answers 2

Reset to default
6

If you want to use functional programming techniques, it's better to refactor your code to be able to construct objects fully with constructors, builders or factory methods. Setters are bad friends as they imply mutability and functional programming likes immutable stuff.

So add new Bar constructor:

public Bar(String fooo, int id) { this.fooo = fooo; this.id = id; }

You may probably realize after this that all your Bar objects can be used without setters. If so, you can simply remove setters and make Bar fields final, so Bar will become immutable. Even if you cannot get rid of setters in other place, having new constructor your foo method can be rewritten in much cleaner way:

public Bar foo(int id) { return repo.stream() .filter(c -> c.getId() == id) .findFirst() .orElseGet(() -> new Bar("", 0)); }
Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

1

Your solution is pretty close to my suggestion:

public Bar foo(int id) { return repo.stream() // find by id .filter(c -> c.getId() == id) // choose any .findAny() .orElseGet(() -> { target = new Bar(); target.setFooo(""); target.setId(0); return target; }); }

Do not create more complex constructors. They are even often refered to as anti-patterns. Stick to your setters!

If you want to optimize let your setters return the updated object. That approach will allow to add any number of setters in future. That would look like that:

public Bar setFooo(String fooo) { this.fooo = fooo; return this; // return modified instance } // do the same for setId()

a bit later in the code:

.orElseGet(() -> new Bar() .setFooo("") .setId(0) // add any number of future setters here );
Improve this answer

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.