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

Poor formatting of multiplication/division of parenthesized expression containing +/- #3552

Open
jturner314 opened this issue May 14, 2019 · 1 comment

Comments

@jturner314
Copy link

When a long parenthesized expression containing addition or subtraction is followed by multiplication or division, the formatted result can be confusing. These are some examples:

fn main() {
    let dv = (2. * m * l * dtheta * dtheta * theta.sin()
        + 3. * m * g * theta.sin() * theta.cos()
        + 4. * u
        - 4. * b * v)
        / (4. * (M + m) - 3. * m * theta.cos().powi(2));
    let ddtheta = (-3. * m * l * dtheta * dtheta * theta.sin() * theta.cos()
        - 6. * (M + m) * g * theta.sin()
        - 6. * (u - b * v) * theta.cos())
        / (4. * l * (m + M) - 3. * m * l * theta.cos().powi(2));
}
fn main() {
    let V: Array2<_> = (((&lq + &vi).mapv(f64::exp) - &q) * (mi_minus_mi_t).mapv(f64::cos)
        - ((&lq - &vi).mapv(f64::exp) - &q) * (mi_plus_mi_t).mapv(f64::cos))
        * e.as_row()
        * e.as_column()
        * 0.5;
}
fn main() {
    dVdm.slice_mut(s![.., .., j]).assign(
        &(((Array2::zeros((d, d)) + u.as_column() - u.as_row()) * &U1
            + (Array2::<f64>::zeros((d, d)) + u.as_column() + u.as_row()) * &U2)
            * e.as_column()
            * e.as_row()),
    );
}
fn main() {
    {
        {
            {
                {
                    {
                        let LdXi: Array2<_> = (dmahai
                            + kdX.slice(s![.., i, d]).into_column()
                            + kdX.slice(s![.., j, d]))
                            * &L;
                    }
                }
            }
        }
    }
}

In all these examples, there is a multiplication or division of a parenthesized expression containing + or -. The * or / is indented by the same amount as the + or -, suggesting that it's at the same level, which by order of operations would mean that only the last expression would be multiplied/divided. It's not obvious that the + or - are in fact inside a parenthesized expression.

A relatively small change that would help would be to increase the indentation of the expressions inside the parentheses, like this:

fn main() {
    let dv = (2. * m * l * dtheta * dtheta * theta.sin()
            + 3. * m * g * theta.sin() * theta.cos()
            + 4. * u
            - 4. * b * v)
        / (4. * (M + m) - 3. * m * theta.cos().powi(2));
    let ddtheta = (-3. * m * l * dtheta * dtheta * theta.sin() * theta.cos()
            - 6. * (M + m) * g * theta.sin()
            - 6. * (u - b * v) * theta.cos())
        / (4. * l * (m + M) - 3. * m * l * theta.cos().powi(2));
}
fn main() {
    let V: Array2<_> = (((&lq + &vi).mapv(f64::exp) - &q) * (mi_minus_mi_t).mapv(f64::cos)
            - ((&lq - &vi).mapv(f64::exp) - &q) * (mi_plus_mi_t).mapv(f64::cos))
        * e.as_row()
        * e.as_column()
        * 0.5;
}
fn main() {
    dVdm.slice_mut(s![.., .., j]).assign(
        &(((Array2::zeros((d, d)) + u.as_column() - u.as_row()) * &U1
                + (Array2::<f64>::zeros((d, d)) + u.as_column() + u.as_row()) * &U2)
            * e.as_column()
            * e.as_row()),
    );
}
fn main() {
    {
        {
            {
                {
                    {
                        let LdXi: Array2<_> = (dmahai
                                + kdX.slice(s![.., i, d]).into_column()
                                + kdX.slice(s![.., j, d]))
                            * &L;
                    }
                }
            }
        }
    }
}

I think a better solution would be to make the parentheses more obvious by adding new lines after the opening parenthesis and before the closing parenthesis, with something like this:

fn main() {
    let dv = (
            2. * m * l * dtheta * dtheta * theta.sin()
            + 3. * m * g * theta.sin() * theta.cos()
            + 4. * u
            - 4. * b * v
        ) / (4. * (M + m) - 3. * m * theta.cos().powi(2));
    let ddtheta = (
            -3. * m * l * dtheta * dtheta * theta.sin() * theta.cos()
            - 6. * (M + m) * g * theta.sin()
            - 6. * (u - b * v) * theta.cos()
        ) / (4. * l * (m + M) - 3. * m * l * theta.cos().powi(2));
}
fn main() {
    let V: Array2<_> = (
            ((&lq + &vi).mapv(f64::exp) - &q) * (mi_minus_mi_t).mapv(f64::cos)
            - ((&lq - &vi).mapv(f64::exp) - &q) * (mi_plus_mi_t).mapv(f64::cos)
        )
        * e.as_row()
        * e.as_column()
        * 0.5;
}
fn main() {
    dVdm.slice_mut(s![.., .., j]).assign(&(
        (
            (Array2::zeros((d, d)) + u.as_column() - u.as_row()) * &U1
            + (Array2::<f64>::zeros((d, d)) + u.as_column() + u.as_row()) * &U2
        )
        * e.as_column()
        * e.as_row()
    ));
}
fn main() {
    {
        {
            {
                {
                    {
                        let LdXi: Array2<_> = (
                            dmahai
                            + kdX.slice(s![.., i, d]).into_column()
                            + kdX.slice(s![.., j, d])
                        ) * &L;
                    }
                }
            }
        }
    }
}

Although doing so is not necessary for clarity, if the closing parenthesis is moved to a new line, we could eliminate some of the extra new lines, so the middle examples would become this:

fn main() {
    let V: Array2<_> = (
            ((&lq + &vi).mapv(f64::exp) - &q) * (mi_minus_mi_t).mapv(f64::cos)
            - ((&lq - &vi).mapv(f64::exp) - &q) * (mi_plus_mi_t).mapv(f64::cos)
        )
        * e.as_row() * e.as_column() * 0.5;
}
fn main() {
    dVdm.slice_mut(s![.., .., j]).assign(&(
        (
            (Array2::zeros((d, d)) + u.as_column() - u.as_row()) * &U1
            + (Array2::<f64>::zeros((d, d)) + u.as_column() + u.as_row()) * &U2
        )
        * e.as_column() * e.as_row()
    ));
}

or this:

fn main() {
    let V: Array2<_> = (
            ((&lq + &vi).mapv(f64::exp) - &q) * (mi_minus_mi_t).mapv(f64::cos)
            - ((&lq - &vi).mapv(f64::exp) - &q) * (mi_plus_mi_t).mapv(f64::cos)
        ) * e.as_row() * e.as_column() * 0.5;
}
fn main() {
    dVdm.slice_mut(s![.., .., j]).assign(&(
        (
            (Array2::zeros((d, d)) + u.as_column() - u.as_row()) * &U1
            + (Array2::<f64>::zeros((d, d)) + u.as_column() + u.as_row()) * &U2
        ) * e.as_column() * e.as_row()
    ));
}
@topecongiro topecongiro added this to the 2.0.0 milestone May 16, 2019
@topecongiro topecongiro modified the milestones: 2.0.0, 3.0.0 Apr 19, 2020
@ytmimi ytmimi added the a-binop label Jul 20, 2022
@ytmimi
Copy link
Contributor

ytmimi commented Jul 20, 2022

Feels like this is related or is a duplicate of #3551

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants