eclipse-archived / golo-lang

Golo - a lightweight dynamic language for the JVM.
http://golo-lang.org/
Eclipse Public License 2.0
476 stars 91 forks source link

How to test null and then do something? The correct way? #392

Open k33g opened 8 years ago

k33g commented 8 years ago

hey 👋 @yloiseau

I have to test if a value is null, if not I do/run something, otherwise do/run something else, but what is the best way to do this?

if response: data() isnt null {
  log(
    "login: {0} email: {1}",
    response: data(): get("login"),
    response: data(): get("email")
  )
} else {
  log("{0} {1}", response: code(), response: message())
}
match {
  when response: data() isnt null then {
    log(
      "login: {0} email: {1}",
      response: data(): get("login"),
      response: data(): get("email")
    )
  }()
  otherwise {log("{0} {1}", response: code(), response: message())}()
}
Option(response: data()): either(|data| {
  log(
    "login: {0} email: {1}",
    data: get("login"),
    data: get("email")
  )
}, {
  log("{0} {1}", response: code(), response: message())
})
case {
  when response: data() isnt null {
    log(
      "login: {0} email: {1}",
      response: data(): get("login"),
      response: data(): get("email")
    )
  }
  otherwise {
    log("{0} {1}", response: code(), response: message())
  }
}
cond(
  |response| -> response: data() isnt null,
  |responseIfTrue| {
    log(
      "login: {0} email: {1}",
      responseIfTrue: data(): get("login"),
      responseIfTrue: data(): get("email")
    )
  },
  |responseIfFalse| {
    log("{0} {1}", responseIfFalse: code(), responseIfFalse: message())
  }
) (response)
yloiseau commented 8 years ago

It's a matter of style and personal preference. However, there are some elements to take into account. From my point of view, it mainly depends on what you want to do, if it's a statement (or side-effect procedure), or an expression (or side-effect free pure function).

Just to be clear, when I say function, I mean a side-effect free, pure computation that returns a value, and thus the call is an expression (something that has a value, ideally with referential transparency); by procedure, I mean an operation that always returns nothing, and has a side-effect (like IO, or changing a state), and thus the call is a statement (has no value, but changes the global state). See also CQS for the same idea.

I tend to use conditional statement when the alternatives are statements, and more functional structures when the alternatives are expressions.

For instance I'd do:

if condition {
    println("plop")
} else {
    println("foo")
}

or

var message = ""
if condition {
    message = "plop"
} else {
    message = "foo"
}
println(message)

but

let message = match {
    when condition then "plop"
    otherwise "foo"
}
println(message)

or even

println(match {
    when condition then "plop"
    otherwise "foo"
})

Note the influence of style :smile:

Since match is an expression, I would not recommend to use it with a procedure (like log).

Since it's here a binary choice (either null or not), I would not use a case.

Option is interesting if you have a function returning such a value, to chain (pure) function applications without checking for nullity. It's the equivalent of elvis (?:) for method applications. The either method should be used with pure functions to provide a final value (as ofIfNull). For instance

let message = functionThatReturnsAnOption()
                : andThen(aPureFunction)
                : andThen(anOtherPureFunction)
                : either(
                  extractAndTransformTheFinalValue,
                  computeTheDefaultValue)
println(message)

I would not create an Option directly from a nullable value just to call either on it, even less if the operations are procedures.

cond is a combinator, and its main purpose is thus to create functions. In your case:

let logValue = cond(`isnt(null), 
  |v| { ... }
  |v| { ... })

logValue(functionReturningNullOrValue())
logValue(anotherFunction()).

However, cond is also meant to be used with/create pure functions

listOfResults: map(cond(`isnt(null), |v| -> v: foo(), |v| -> "default value"))

Don't forget the last option you didn't mention: elvis ?: and orIfNull (although it would be a little cumbersome to use in your use case).

Finally, algebraic data type and polymorphism can also be used to leverage this problem:

union Data = {
    User = { login, email }
    Error = { code, message }
}

augment Data$User = {
    function logInfo = |this| -> "login: %s email: %s": format(
        this: login(),
        this: email())
}

augment Data$Error = {
    function logInfo = |this| -> "%s %s": format(
        this: code(),
        this: message())
}

...
response = UserData("fperfect", "ford.perfect@h2g2.org")
# or
response = Error(42, "Unknown user")
# You never have `null` values
...

log(response: logInfo())

or if you don't want to augment your data with logging concerns

function logInfo = |v| -> match {
  when v: isUser() then "login: %s email: %s": format(
        this: login(),
        this: email())
  when v: isError() then "%s %s": format(
        this: code(),
        this: message())
  otherwise raise("Unknown data type")
}

...

log(logInfo(response))

As a side note, it's often recommended to not use negative conditional, and thus prefer if result is null over if result isnt null.

k33g commented 8 years ago

@yloiseau crystal clear

yloiseau commented 8 years ago

That being said, it's also a matter of style and personal preferences, and even if I have rational arguments, someone else could argue the other way around :smile: