I was browsing some old code the other days because it needed a few updates and I stumbled upon this function:

is_not_null_na <- function(value)
{
  !is.null(value) && !is.na(value)
}

Actually, this function had a much longer name, which I find unnecessary for such a small function. The next thing that bothered me were the used negations. To my knowledge, the human brain has great problems in dealing with negations (mine certainly does). As explained for example here, as soon as you tell the brain "don't think of an elephant", the first thing that the brain thinks of is of course an elephant...

In software things get more complicated with time. For example, it is recommended to start with the positive conditionals:

# bad
if (!file.exists(some_file_name))
{
  # do something
}
else
{
  # do some other thing
}

#------------------------------------
# good
if (file.exists(some_file_name))
{
  # do something
}
else
{
  # do some other thing
}

Now imagine the case in which we throw an error if the file is missing. In this case is of course perfectly fine to do this:

# good
if (!file.exists(some_file_name))
{
  stop("file '", some_file_name, "' does not exist"
}
# do something else

#---------------------------------------
# also good
is_missing_file <- !file.exists(some_file_name)
if (is_missing_file)
{
  stop("file '", some_file_name, "' does not exist"
}
# do something else

The problem might come at a later stage, for example the requirements might change. Let's say the file must be there, but we also must ensure this is a directory. One might be tempted to write this:

is_missing_file <- !file.exists(some_file_name)
is_directory <- utils::file_test(op = "-d", some_file_name)
if (!is_missing_file && is_directory)
{
  # do something
}
else
{
  # do something else
}

You just entered the vicious circle of double negations... (of course there are simpler ways to check if a directory exists - this is just a contrived example to prove a point). You might think that this is not so bad. Believe me, things can get worse.

Let me demonstrate with the function I started with. Do you remember it?

is_not_null_na <- function(value)
{
  !is.null(value) && !is.na(value)
}

I searched in the code to see where this was actually used. This is what I found:

if (obj$type == "logical")
{
  if (!( is_not_null_na(obj$operator_name) &&
       obj$operator_name %in% c("lte", "gte")) &&
    !(!is_not_null_na(obj$variable_name) || obj$variable_name == "value") )
  {
    obj$comparison_value <- NA_real_
  }
} 
else 
{
  obj$variable_id <- NA_integer_
  obj$variable_name <- NA_character_
  obj$function_id <- NA_integer_
  obj$function_name <- NA_character_
  obj$comparison_value <- NA_real_
}

Have you noticed the triple negation? Let me write it down again for you:

!(!is_not_null_na(obj$variable_name)

In that moment I was trying desperately to remember how to breath, as I realized that my brain just doesn't have the necessary equipment to handle this load... But maybe the meaning of the triple negation above is obvious to you? If so, please let me know.

I found here a nice visual explanation of what happens in the brain when it sees negations. I agree with most of the statements in that article. Exception: I would rewrite the last example (and I don't share the opinion about the car):

# bad
if (!isCar || !isElectric || !isFast || !isAwesome)
{
  isTesla <- FALSE
}
else
{
  isTesla <- TRUE
}

# also bad
if (isCar && isElectric && isFast && isAwesome)
{
  isTesla <- TRUE
}
else
{
  isTesla <- FALSE
}

# good
isTesla <- isCar && isElectric && isFast && isAwesome

After spending (too much) time with the code above, here are my attempts to simplify it.

First try:

if (obj$type == "logical")
{
  operators <- c("lte", "gte")
  has_comparison_operator <- isTRUE(obj$operator_name %in% operators)
    
  if (!has_comparison_operator &&
      !( !is_not_null_na(obj$variable_name) || obj$variable_name == "value") )
  {
    obj$comparison_value <- NA_real_
  }
} 
else 
{
  obj$variable_id <- NA_integer_
  obj$variable_name <- NA_character_
  obj$function_id <- NA_integer_
  obj$function_name <- NA_character_
  obj$comparison_value <- NA_real_
}

Second try:

if (obj$type == "logical")
{
  operators <- c("lte", "gte")
  has_comparison_operator <- isTRUE(obj$operator_name %in% operators)
    
  if (!has_comparison_operator &&
      !( is.na(obj$variable_name) || is.null(obj$variable_name) || obj$variable_name == "value") )
  {
    obj$comparison_value <- NA_real_
  }
} 
else 
{
  obj$variable_id <- NA_integer_
  obj$variable_name <- NA_character_
  obj$function_id <- NA_integer_
  obj$function_name <- NA_character_
  obj$comparison_value <- NA_real_
}

Third try:

obj$comparison_value <- NA_real_

operators <- c("lte", "gte")
has_comparison_operator <- isTRUE(obj$operator_name %in% operators)

if (obj$type != "logical" && has_comparison_operator &&
  (is.na(obj$variable_name) || is.null(obj$variable_name) || obj$variable_name == "value")
) {
  obj$variable_id <- NA_integer_
  obj$variable_name <- NA_character_
  obj$function_id <- NA_integer_
  obj$function_name <- NA_character_
  obj$comparison_value <- NA_real_
}

Fourth try:

get_verified_obj <- function(rule)
{
  obj$comparison_value <- NA_real_
  
  if (obj$type == "logical")
  {
    return(obj)
  }
    
  operators <- c("lte", "gte")
  has_comparison_operator <- isTRUE(obj$operator_name %in% operators)
    
  if (!has_comparison_operator)
  {
    return(obj)
  }
    
  if (is.na(obj$variable_name) || is.null(obj$variable_name) || obj$variable_name == "value")
  {
    obj$variable_id <- NA_integer_
    obj$variable_name <- NA_character_
    obj$function_id <- NA_integer_
    obj$function_name <- NA_character_
    obj$comparison_value <- NA_real_
  }
    
  return(obj)
}

Unfortunately, by trying to re-write this code for mere mortals the risk is high to do the wrong thing (by the way, can you tell me if I did it wrong?)

While I respect legacy code (in the end, without legacy code I wouldn't even get paid today), I also hope that by giving this little piece of advice - do write positive code -, code can get if not better, at least easier to understand and to maintain.

You can find more discussions on negative conditions in programming in general here. What is your opinion about this? Do you care? If yes, feel free to leave a comment.

Make a promise. Show up. Do the work. Repeat.