Java init object and set property using Stream

Question

I'm trying to clone a list to a new list and set a property in the new list. I'm trying to use Java8 Stream as it makes cloning simple. My code works but it gives this code smell from Sonar:

Local variables should not be declared and then immediately returned or thrown (squid:S1488)

Is there a way to do this without using a local variable? code:

List<myObject> clonedList = listToClone.stream()
                                                .map(item ->  {
                                                    cloned = new myObject(item);
                                                    cloned.setLifeCycle("someLifeCycle");
                                                    return cloned;
                                                })
                                                .collect(Collectors.toList());

Thanks

I am not sure that your problem is on the map method. Can you show the rest of the method. The problem I have with your map method is that the variable is not declared in the map method. I always use the smallest scope possible. But it is not your problem here — Sodala, Nov 12 '18 at 10:17
@Sodala On the other hand, if cloned isnt a local variable, this wouldnt compile. Strange. — GhostCat, Nov 12 '18 at 10:20
Please read "How to create a [mcve]". Then use the [edit] link to improve your question (do not add more information via comments). Otherwise we are not able to answer your question and help you. — GhostCat, Nov 12 '18 at 10:20
This answer refers to filter but is basically the same: https://stackoverflow.com/a/45793560/9354242 — Meini, Nov 12 '18 at 10:36

score 1 · Accepted Answer · answered Nov 12 '18 at 10:16

1

It is a warning because you have used a new variable cloned unnecessarily instead of directly chaining functions like

List<myObject> clonedList = listToClone.stream()
    .map(item -> {return (new myObject(item)).setLifeCycle("someLifeCycle");})
    .collect(Collectors.toList());

answered Nov 12 '18 at 10:16

Mani Kumar Reddy Kancharla

93
10

1

or just `.map(item -> new myObject(item).setLifeCycle("someLifeCycle"))` assuming `setLifeCycle` doesnt return `void`. – Ousmane D. Nov 12 '18 at 10:17

score 1 · Answer 2 · answered Nov 12 '18 at 10:23

1

You can try this:

List<myObject> clonedList = listToClone.stream()
                                       .map(myObject::new)
                                       .map(o -> {
                                           o.setLifeCycle("someLifeCycle");
                                           return o;
                                       })
                                       .collect(Collectors.toList());

answered Nov 12 '18 at 10:23

ETO

6,970
1
20
37

I wouldn't ignore the fact that the function to map should be "stateless function to apply to each element". – Ousmane D. Nov 12 '18 at 10:24
1

@Aomine Well, it would be cleaner to collect the new objects and then call `forEach` on clonedList setting the properties we need. The question was whether we *can do this without a local variable*. – ETO Nov 12 '18 at 10:30
1

"The question was whether we can do this without a local variable". so wouldn't it be _neater_ to do `listToClone.stream() .map(myObject::new).peek(o -> o.setLifeCycle("someLifeCycle")).collect(toList())` which "yes" is just as bad as having a statefuly `map` function because `peek` should be used for debugging purposes only. anyway, if you're going to suggest this code then atleast state the probems it could cause. the original code shown by OP will work perfectly both sequentially and in parallel, unlike your solution. – Ousmane D. Nov 12 '18 at 10:35
Dont use peek for such reasons it should be used only for debugging thus java documentation says us : https://docs.oracle.com/javase/8/docs/api/java/util/stream/Stream.html#peek-java.util.function.Consumer- – Mykhailo Moskura Nov 12 '18 at 10:38
@Aomine Why isn't it safe working in parallel? The mapping for each item of the initial list will be executed sequentially anyway. – ETO Nov 12 '18 at 10:52

Mykhailo Moskura · Answer 3 · 2018-11-12T10:30:40.300

0

        public class MyObject{
         private String someLifeCycle;
         private Item item;
          public MyObject(final String someLifeCycle,final Item item){
            this.someLifeCycle = someLifeCycle;
            this.item = item;
           }
          //getters and setters
        }

And your code will be like this :

    List<MyObject> clonedList = listToClone.stream()
 .map(item -> new MyObject(item,"someLifeCycle")).collect(Collectors.toList());

edited Nov 12 '18 at 10:30

answered Nov 12 '18 at 10:21

Mykhailo Moskura

2,073
1
9
20

I would give +1 for the UnaryOperator thing, which I havent seen yet. But then -1 for just dropping a bunch of (not so nicely indented) code. So, zero sum game. – GhostCat Nov 12 '18 at 10:26
@GhostCat which doesnt even compile. – Ousmane D. Nov 12 '18 at 10:28
@Aomine what doesnot compile? – Mykhailo Moskura Nov 12 '18 at 10:29
@MykhailoMoskura `final someLifeCycle` – Ousmane D. Nov 12 '18 at 10:30
Sorry , thats was typo i was typing fast – Mykhailo Moskura Nov 12 '18 at 10:30
@Aomine There is one more thing , you can use builder from lombok and in that case you dont nee a constructor – Mykhailo Moskura Nov 12 '18 at 10:37

Java init object and set property using Stream

3 Answers3