Benefits of isFALSE over !

[m7pr]

Hey, I'm new in the project and in the code and some conventions are not year clear for me. I allowed myself to open this small discussion, where everybody can express their opinion or replies offline/asynchronously (in their own time, on their own pace). This is not a big deal so do not feel you are obliged to provide any complex and broad explanations - do not even feel obliged to participate at all. Reply only if you think it's worth it.

I wanted to discuss the topic of the usage of isFALSE() and isTRUE within teal.slice code against the typical usage of ! and lack of it for the verification of logical flag statuses.

Imagine the below code

x <- TRUE
if(x == TRUE) {
  cat(5)
}

in which == TRUE is redundant, as the same can be achieved with

x <- TRUE
if(x) {
  cat(5)
}

and it's faster and uses less space. There is one more function to detect logical values which is isTRUE() and isFALSE() that you could use in this situation

x <- TRUE
if(isTRUE(x)){
  cat(5)
}

which seems as overcomplicated and as unnecessary as == TRUE .

Is there any specific reason we use isTRUE() and isFALSE() within the code besides it's a bit more than just identical(x, TRUE).
It is actually is.logical(x) && length(x) == 1 && !is.na(x) && x.
Is this needed because of the shiny or specific states of elements of our classes?

Happy to hear the reasoning, as perhaps there is any. Eager to get it in writing for future newcomers as well as a small reference on a design decisions.

[chlebowa]

The cases I know of are when the tested value need not be a boolean at all. Try these:

x <- NULL
if (x) print("ok") else print("not ok")

x <- all.equal(letters[1:4], letters[4:1])
if (x) print("ok") else print("not ok")

I think there are also cases of empty logical vecotrs.

if (logical(0)) print("ok") else print("not ok")

[m7pr]

Ok thanks, so the verification of isTRUE and isFALSE is broader but is it needed. It is mainly used on the `$is_disabled()` method of `FilterState`. Do you think it is possible that this method returns anything else besides TRUE of FALSE?

…

On Fri, May 26, 2023 at 2:39 PM Aleksander Chlebowski < ***@***.***> wrote: The cases I know of are when the tested value need not be a boolean at all. Try these: x <- NULL if (x) print("ok") else print("not ok") x <- all.equal(letters[1:4], letters[4:1]) if (x) print("ok") else print("not ok") I think there are also cases of empty logical vecotrs. if (logical(0)) print("ok") else print("not ok") — Reply to this email directly, view it on GitHub <#303 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/A74AIEK4IFO2AYCMCZFVFZLXICP7RANCNFSM6AAAAAAYQEQADM> . You are receiving this because you authored the thread.Message ID: <insightsengineering/teal.slice/repo-discussions/303/comments/6011375@ github.com>

[chlebowa]

Actually, in that particular case it might be overkill. However, I notice that filter states are enabled during app start, I suspect because the disable switch is created with value = TRUE or because the constructor sets the initial value of private$disabled to reactiveVal(TRUE). I decided to add it just in case at some point, thinking the potential overhead won't be too large.

[m7pr]

ok, this makes more sense. let's investigate someday if this is not too much. for now closing the discussion