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

Proposal: Multi-object if #8351

Closed
Mouvedia opened this issue Mar 24, 2021 · 6 comments
Closed

Proposal: Multi-object if #8351

Mouvedia opened this issue Mar 24, 2021 · 6 comments
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@Mouvedia
Copy link

Mouvedia commented Mar 24, 2021

In #7257, which has been accepted, @thejoshwolfe says:

The discussion of generalizing this idea to other syntactic constructs is interesting, but those ideas should be their own issue/proposal if they are going to be accepted.

status quo

const one: ?u32 = 1;
const two: ?u32 = 2;

// the second if is an expr hence it requires a ;
if (one) |foo| if (two) |bar| {
  // …
};

proposal

const one: ?u32 = 1;
const two: ?u32 = 2;

if (one, two) |foo, bar| {
  // …
}

It's pretty much straightforward to me, but Ill add that it is obviously a AND (not a OR) so you will reach the else if at least one of the two is null.
Of course the order will have to be matched for the else captures:

    const a: anyerror!u32 = 0;
    const b: anyerror!u32 = error.BadValue;

    if (a, b) |foo, bar| {
        unreachable;
    } else |_, err| {
        expect(err == error.BadValue);
    }

The only downsides that I see is if the if body is huge it will require a back and forth to check the order which can induce errors but on the other hand it will also promote the good practice of refactoring your code into smaller chunks (so that you see both the if and the else).

@alexnask alexnask added the proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. label Mar 24, 2021
@Guigui220D
Copy link

It doesn't add much to me, it's arguably less readable and adds a way of doing things and a thing to know about the langage

@Vexu Vexu added this to the 0.8.0 milestone Mar 24, 2021
@ghost
Copy link

ghost commented Mar 24, 2021

This is pure syntax sugar, which I might add will make it easier in most cases to do the wrong thing. (When are you ever going to want to check multiple optionals and nothing else?)

I don't think you quite understand the philosophy of Zig. We do not add features just for the sake of it, or for symmetry or whatever -- only ever to satisfy some concrete use case, or to make it easier to do things correctly. This is neither.

@ghost
Copy link

ghost commented Mar 24, 2021

@Mouvedia,
your second example will only work if all errors except the selected one are silently ignored, i.e., the else clause is not entered at all if one of the other tests fails. I think this is too much of a footgun, not to mention inconsistent with how the same construct works on optionals.

If restricted to optionals only, then this proposal would be an improvement in readability, IMO. But the layer of syntax sugar is pretty thin and the use case pretty rare to begin with, so all in all this seems to be more trouble than it's worth.

@Mouvedia
Copy link
Author

Mouvedia commented Mar 24, 2021

It seemed pretty inconsistent to me. With so many 👎 Ill just close it.

@yohannd1
Copy link

yohannd1 commented Apr 9, 2021

I have an example where this could be pretty useful, but I'm also not sure if it's common enough that it would be a good idea to add it to the language.
Basically it's when you need to get more than one optional value (it might apply to errors to but I haven't thought very well about that one).

test "currently - first way" {
    const opt1: ?i32 = null;
    const opt2: ?i32 = null;

    if (opt1) |o1| {
        if (opt2) |o2| {
            // <DO SOMETHING WITH: o1 and o2>
        } else {
            // This could potentially be a big block and copying it would be inconvenient:
            // <ELSE CODE>
        }
    } else {
        // This is the same block as the <ELSE CODE> above.
        // <ELSE CODE>
    }
}

test "currently - second way" {
    const opt1: ?i32 = null;
    const opt2: ?i32 = null;

    blk: {
        if (opt1) |o1| {
            if (opt2) |o2| {
                // <DO SOMETHING WITH: o1 and o2>
                break :blk;
            }
        }

        // <ELSE CODE>
    }
}

test "with this proposal" {
    const opt1: ?i32 = null;
    const opt2: ?i32 = null;

    if (opt1, opt2) |o1, o2| {
        // <DO SOMETHING WITH: o1 and o2>
    } else {
        // <ELSE CODE>
    }
}

Using bool along with optional types

This could also work with bools, but then I have no idea how the syntax would be:

const b: bool = false;
const opt: ?i32 = null;

if (b, opt) |_, o| {} // this looks really strange
if (b, opt) |o| {} // omitting the bool's ignored capture - even stranger

if (b) andthen (opt) |o| {} // this might be interesting but it would be yet another strange syntax construct

Yet another suggestion

There's also another syntax suggestion I have previously suggested on a comment of another issue that could be tailored so it can be used with bools too.

const b: bool = false;
const opt: ?i32 = null;

b andthen opt andthen |o| {}

This still doesn't fix the issue the return type of the expression is kind of vague (does [bool value] andthen [?i32 value] return a ?i32, a bool, or what?), though. Maybe just discarding this multi-type if/andthen part I suggested is best.

@ghost
Copy link

ghost commented Apr 9, 2021

@YohananDiamond
If large else clauses need to be merged, that can also be done the C-way, by setting an external flag:

test "currently - third way" {
    const opt1: ?i32 = null;
    const opt2: ?i32 = null;

    var success = false;
    if (opt1) |o1| if (opt2) |o2| {
        success = true;
        // Do something with o1 and o2
    }
    if (!success) {
        // Big block of shared else code
    }
}

Maybe not the most robust of solutions, but doesn't require new syntax and can be easily adapted to more complex control flow e.g., if you need to test a boolean, an optional and an error at the same time, while this proposal would only allow composing multiple optionals.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
Development

No branches or pull requests

5 participants