Third time using && { ...; true } for side effects

21

The expression if thing { true } else { false } is only redundant if there's nothing else in the arms of the if. And if you still insist on not writing that, then introducing a variable is not "unnecessary", it's the simplest solution.

That's not better at all. It's completely inconsistent with expressions just being expressions, and the current syntax of conditionals (including if). The discriminant of conditional expressions is not the final value of the conditional expression, the proposed syntax is borderline mind-boggling.

5 Likes
22

It's possible that a better option would be for you.. collectively.. to make a new language with a preprocessor that implements some of these enhancements. This specific case might be better served by a new keyword, e.g. if { RETURNING foo } { ... } ala SQL.

1 Like
23

These are exactly equivalent in return position:

some_bool?;
some_side_effects;
true

and

some_bool && {
  some_side_effects;
  true
}

but the former is "magically" more clear?

(fwiw, we'd like to get rid of the true while at it, as we consider it frivolous.)

24

If you want to get rid of the true then why is storing the value in a binding "unnecessary"?

25

Because then you have 3 occurrences of binding

let binding = ...;
if binding { ... }
binding
26

Here's a more interesting example, consider this:

    /// predicate <- ':' ( '?' -> MarkSkippable ) '$' ( identifier -> Predicate )
    fn predicate(&mut self, s: &mut &'s str) -> Result<bool, PatternError<'s>> {
        let mut cursor = *s;
        Ok(
            strip_prefix(&mut cursor, ":")
            &&
            {
                let skippable = strip_prefix(&mut cursor, "?");
                strip_prefix(&mut cursor, "$")
                &&
                {
                    let start = cursor;
                    self.identifier(&mut cursor)?
                    &&
                    {
                        let name = &start[..pos_of(start, cursor).unwrap()];
                        // TODO move predicate from self.preds into
                        // self.consts.predicates and add id to self.pred_ids
                        let id = todo!();
                        let proto = self.consts.protos.last_mut().expect("protos");
                        proto.push(PatternElement::ApplyPredicate(id, skippable, std::marker::PhantomData));
                        *s = cursor;
                        true
                    }
                }
            }
        )
    }

It would be nice if it could be instead:

    /// predicate <- ':' ( '?' -> MarkSkippable ) '$' ( identifier -> Predicate )
    fn predicate(&mut self, s: &mut &'s str) -> Result<bool, PatternError<'s>> {
        let mut cursor = *s;
        Ok(
            'matches: {
                strip_prefix(&mut cursor, ":") 'matches?;
                let skippable = strip_prefix(&mut cursor, "?");
                strip_prefix(&mut cursor, "$") 'matches?;
                let start = cursor;
                self.identifier(&mut cursor)? 'matches?;
                let name = &start[..pos_of(start, cursor).unwrap()];
                // TODO move predicate from self.preds into
                // self.consts.predicates and add id to self.pred_ids
                let id = todo!();
                let proto = self.consts.protos.last_mut().expect("protos");
                proto.push(PatternElement::ApplyPredicate(id, skippable, std::marker::PhantomData));
                *s = cursor;
                true
            }
        )
    }

And lib-based solutions would provide forward drift, which is less than ideal in this case.

A macro-based solution can work tho:

    /// predicate <- ':' ( '?'? -> MarkSkippable ) '$' ( identifier -> Predicate )
    fn predicate(&mut self, s: &mut &'s str) -> Result<bool, PatternError<'s>> {
        let mut cursor = *s;
        Ok(lblock!('matches: {
            bry!('matches strip_prefix(&mut cursor, ":"));
            let skippable = strip_prefix(&mut cursor, "?");
            bry!('matches strip_prefix(&mut cursor, "$"));
            let start = cursor;
            bry!('matches self.identifier(&mut cursor)?);
            let name = &start[..pos_of(start, cursor).unwrap()];
            // TODO move predicate from self.preds into
            // self.consts.predicates and add id to self.pred_ids
            let id = todo!();
            let proto = self.consts.protos.last_mut().expect("protos");
            proto.push(PatternElement::ApplyPredicate(id, skippable, std::marker::PhantomData));
            *s = cursor;
            true
        }))
    }
27

It can also be done without the variable if you use an early return:

if !something_else.mutate() {
  return false;
}
something = something_else;
true

Personally, I generally think it's an antipattern to use logic operators for control flow, but if you must use logic operators instead of if/else, you can write it like this:

something_else.mutate() || return false;
something = something_else;
true

But if you find yourself doing this a lot, it's probably best to take some approach to avoid it (maybe through Try, maybe macros, etc). In fact, I would go so far as to say: using a lot of bools at all is an antipattern, because it's possible to mix up the sense of true/false (e.g. does HashSet::insert() return true when it finds an existing element, or return true when it finds an empty spot to insert into? I think I remember, but would I bet my life on it?) Instead, I'd strongly consider either using Result or writing a custom enum type where the variant names make it clear what each of them means.

2 Likes
28

That looks like if_chain?

    fn predicate(&mut self, s: &mut &'s str) -> Result<bool, PatternError<'s>> {
        let mut cursor = *s;
        Ok(if_chain!{
            if strip_prefix(&mut cursor, ":");
            let skippable = strip_prefix(&mut cursor, "?");
            if strip_prefix(&mut cursor, "$");
            let start = cursor;
            if self.identifier(&mut cursor)?;
            let name = &start[..pos_of(start, cursor).unwrap()];
            // TODO move predicate from self.preds into
            // self.consts.predicates and add id to self.pred_ids
            let id = todo!();
            let proto = self.consts.protos.last_mut().expect("protos");
            then {
                proto.push(PatternElement::ApplyPredicate(id, skippable, std::marker::PhantomData));
                *s = cursor;
                true
            } else { false }
        })
    }
3 Likes
29

No this is try on bools.

30

Given that NoneError isn't slated to being stabalized (prefering ok_or and ok_or_else) I doubt that try will be implemented on bool for return values that are not also bool.

(At least this is what my reading of the Try Trait V2 RFC is (which hasn't even been merged yet))

31

It's fine, we want to use it with try/labeled blocks. They return a bool, so try-bool-to-bool should be fine.

32

I just mentioned that because of your example above (which had the return value -> Result<bool, ...>

33

Early returns seem just fine for this example:

/// predicate <- ':' ( '?' -> MarkSkippable ) '$' ( identifier -> Predicate )
fn predicate(&mut self, s: &mut &'s str) -> Result<bool, PatternError<'s>> {
    let mut cursor = *s;
    if !strip_prefix(&mut cursor, ":") {
        return Ok(false);
    }
    let skippable = strip_prefix(&mut cursor, "?");
    if !strip_prefix(&mut cursor, "$") {
        return Ok(false);
    }
    let start = cursor;
    if !self.identifier(&mut cursor)? {
        return Ok(false);
    }
    let name = &start[..pos_of(start, cursor).unwrap()];
    // TODO move predicate from self.preds into
    // self.consts.predicates and add id to self.pred_ids
    let id = todo!();
    let proto = self.consts.protos.last_mut().expect("protos");
    proto.push(PatternElement::ApplyPredicate(id, skippable, std::marker::PhantomData));
    *s = cursor;
    Ok(true)
}
34

Do we have labeled try{} on nightly yet?

35

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.