https://kotlinlang.org logo
#detekt
Title
# detekt
z

zmunm

08/05/2020, 3:56 AM
Hi. I need help.. There are some code like this
Copy code
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..
n

nkiesel

08/05/2020, 4:38 AM
would
Observable.something().map { mutableList.addAll(list) }
not be enough
z

zmunm

08/05/2020, 4:46 AM
In this case, addAll returns Unit
g

gammax

08/05/2020, 5:06 AM
.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.
z

zmunm

08/05/2020, 5:21 AM
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
Copy code
Observable.something()
    .map {
        Person("Adam").apply {
            age = 32
            city = "London"        
        }
    }
Is there any flag? I can't found about that..
g

gammax

08/05/2020, 5:22 AM
Is there any flag?
What do you mean?
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.
z

zmunm

08/05/2020, 5:37 AM
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?
g

gammax

08/05/2020, 5:42 AM
Is there any difference in
also, apply
except
it, this
No there are no other difference other the one you mentioned.
b

Brais Gabin

08/07/2020, 11:33 AM
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.
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.
g

gammax

08/07/2020, 12:18 PM
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
b

Brais Gabin

08/07/2020, 12:19 PM
It does,
doOnNext
I created https://github.com/detekt/detekt/issues/2938 so we can talk if it is or it is not a false positive
👍 3
g

gammax

08/07/2020, 12:22 PM
I’m really rusty with RxJava, but would this be equivalent? 🤔
Copy code
Observable.something()
    .doOnEach {
        mutableList.addAll(it)
    }
b

Brais Gabin

08/07/2020, 1:03 PM
No, you still emit it instead of the updated mutableList
2 Views