Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

language: add else clause as part of if comprehension #2122

Open
myitcv opened this issue Nov 22, 2022 · 4 comments
Open

language: add else clause as part of if comprehension #2122

myitcv opened this issue Nov 22, 2022 · 4 comments
Labels
FeatureRequest New feature or request

Comments

@myitcv
Copy link
Member

myitcv commented Nov 22, 2022

Is your feature request related to a problem? Please describe.

One of the more frequently asked questions on Slack and GitHub discussions is why else is missing from CUE (an incomplete list):

See also #571, regarding not() builtin.

In #111 (comment), @mpvl does not rule out adding else, saying:

It is not missing, it was deliberately omitted. I’m not entirely against adding it, but only after getting more data and experience with comprehensions and seeing how they develop. Conditional expression tend to lead to awful configurations, and although comprehensions are grammatically not expressions, they are getting close and just might develop into them.

Raising this issue to capture more data, experience, feedback etc, because right now there is not an open issue to capture such responses.

Describe the solution you'd like

Adding support for else clauses in if comprehensions.

Describe alternatives you've considered

Per #2092, explicitly stating the negation of the if clause, which leads to errors.

Additional context

n/a

@myitcv myitcv added FeatureRequest New feature or request Triage Requires triage/attention labels Nov 22, 2022
@myitcv myitcv removed the Triage Requires triage/attention label Nov 23, 2022
@verdverm
Copy link

verdverm commented Nov 23, 2022

Would this also imply the need for else if?

if { } [else if {} ...] else {}


I wonder about code like below, where someone is not aware of defaults and uses branching conditionals (if as proposed), rather than conditional guards (as if is today)

foo: string
wasSet: bool

// does this get evaluated twice? will wasSet then always have a conflict?
if foo != _|_ {
  wasSet: true
} else {
  wasSet: false
  foo: "bar"
}

// does this get evaluated before or after the else?
if foo == _|_ {
  cow: "moo"
}

@mpvl
Copy link
Member

mpvl commented Nov 25, 2022

In #111 (comment), @mpvl does not rule out adding else,

Now we have more data and know of several designs, in addition to considering future language extensions, I'm much more leaning to not supporting an else construct now. A switch statement construct is more general and often results in better code.

Also, for now you can simulate a switch statement as such:

a: [ // switch
    if foo != _|_ {
        wasSet: true
    },
    {
        wasSet: false
        foo: "bar"
    }
][0]

This avoids the problem, even now, of having to negate the if condition.

The benefit of this switch construct is that you can combine it with other forms of conditionals. For instance, a common construct is

if expr != _|_ {
    a: expr
}

This is unnecessarily verbose. With the query extension, we foresee being able to instead write something like

a: <- expr // syntax tbd

The switch approach naturally allows for "else" values even in this case. A dedicated else construct does not provide solution for this very common case. Another benefit of the switch approach is that it discourages the use of the nested if anti pattern. We see the benefit of using switch instead of a bunch of if-else statement in languages like Go as well.

As said, you can use this "switch" idiom even in today's CUE. But we plan to have a bit more syntactic support for this, for instance:

a: switch([
    if foo != _|_ {
        wasSet: true
    },
    {
        wasSet: false
        foo: "bar"
    }
])

This better signals intend and will allow cue vet to detect "unreachable" elements.

I would also not be opposed to having a more dedicated language construct that handles this, although I'm not sure how this would look like if one wants to cover all cases. Maybe something like, using the more precise builtins

a: switch {
    case isdefined(foo):
        wasSet: true
    case X=expr; isvalid(X): // a possible way to not have to repeat expr, I guess we could allow this in `if` too.
        X
    default:
        wasSet: false
        foo: "bar"
}

@seh
Copy link
Contributor

seh commented Nov 25, 2022

I like where this is going, though needing to supply an array to the penultimate switch above is onerous, even if it would allow existing source code highlighters and formatters to play along more easily. For the last proposed form above, is there any precedent for a keyword-prefixed block like that in CUE, not using a function call-like syntax?

More potential prior art to consider: Common Lisp's cond and case macros.

@myitcv myitcv added zGarden and removed zGarden labels Jun 13, 2023
@networkhermit
Copy link

Personally I hope there is a else construct.

Manually negating the if condition like the following snippet does is error prone.

if a || b == 0 {
    c: "category a"
}
if !a && b != 0 {
    c: "category b"
}
...

I'm not sure if this kind of usage can be covered by a switch construct.

And I'm a little worried about the performance implication without a else construct to short-circuit the evaluation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FeatureRequest New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants