Hi. I need help.. There are some code like this ```Observable.something() .map { mutabl...

# detekt

[zmunm]

Hi. I need help.. There are some code like this

Observable.something()
    .map {
        mutableList.apply {
            addAll(list)
        }
    }

Console says UnnecessaryApply .. But I need return MutableList I don't know if this is really wrong. How can I fix it? Thank you for reading..

[nkiesel]

would Observable.something().map { mutableList.addAll(list) } not be enough

[zmunm]

In this case, addAll returns Unit

[gammax]

.also{} will do the same job, will not be flagged and will also be a better fit here imho as you’re having a side effect.

[zmunm]

https://kotlinlang.org/docs/reference/scope-functions.html#apply

Thank you for your response.. When I read this document, I think that apply is more correct it this case.
How about this case

Observable.something()
    .map {
        Person("Adam").apply {
            age = 32
            city = "London"        
        }
    }

Is there any flag? I can't found about that..

[gammax]

Is there any flag?

What do you mean?

[gammax]

UnnecessaryApply is designed to flag if you use apply for only one statement. Imho .apply{} is useful when you’re configuring an instance and you want to mutate several of its fields.
Your case instead is applying a side effect on your mutableList and returning it.

[zmunm]

Is there any flag? mean

Is there any difference in also, apply except it, this

Do you mean that addAll is a function, which can cause side effect? It understood.

I also have second case like Person().apply{ age = 32 }
It would be happy if age went into the constructor, but it is difficult to do so.
Is it better to use aslo if there is only one line even for field set?

[gammax]

Is there any difference in also, apply except it, this

No there are no other difference other the one you mentioned.

[Brais Gabin]

I think that this is a false positive... If we are using the return value it's nothing wrong with using apply. As far as I see in the tests we are not testimg this case and we should.

[Brais Gabin]

This code is not a side effect so also doesn't seem a better option for me... I mean, I don't like this code that much because you should not use mutable objects in a stream but that's out of the scope of detekt and the issue.

[gammax]

I don’t like this code that much because you should not use mutable objects in a stream but that’s out of the scope of detekt and the issue.

Agree. That’s also the root cause of the problem actually. Ideally RxJava would have an operator like onEach or similar

[Brais Gabin]

It does, doOnNext

[Brais Gabin]

I created https://github.com/detekt/detekt/issues/2938 so we can talk if it is or it is not a false positive

[gammax]

I’m really rusty with RxJava, but would this be equivalent? 🤔

Observable.something()
    .doOnEach {
        mutableList.addAll(it)
    }

[Brais Gabin]

No, you still emit it instead of the updated mutableList