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

Code style: separate parts of If statement #541

Closed
maxitg opened this issue Dec 2, 2020 · 0 comments · Fixed by #596
Closed

Code style: separate parts of If statement #541

maxitg opened this issue Dec 2, 2020 · 0 comments · Fixed by #596
Labels
refactor Does not change functionality, but makes the code better organized or more readable wolfram language Requires Wolfram Language implementation

Comments

@maxitg
Copy link
Owner

maxitg commented Dec 2, 2020

The problem

The issue with If in Wolfram Language is that it's very easy to miss the difference between a compound expression and an "else". That is, compare

If[x > 0,
  a = 4;
  b = a + 1
]

to

If[x > 0,
  b = 4,
  b = a + 1
]

Possible solution

To make these cases more readable, the following style is suggested:

If[x > 0,
  b = 4
,
  b = a + 1
]

The idea behind this indentation is that the comma takes the part of an else if in a language like C++:

If[x > 0,
  b = 4
,  (* else *)
  b = a + 1
]

A similar style should be used in other functions where compound expressions are expected. For example, Catch:

Catch[
  Message[abc::def];
  x = 5;
,
  Exit[1]
]

Additional context

This should eventually make its way into linter #247. In the meantime, we need to update the existing code itself and the contributing guidelines.

@maxitg maxitg added refactor Does not change functionality, but makes the code better organized or more readable wolfram language Requires Wolfram Language implementation labels Dec 2, 2020
daneelsan added a commit that referenced this issue Jan 27, 2021
## Changes

* Closes #541.
* Format code style of `If`, `Catch`, `Check`, `Quiet` functions.
* Contributing guidelines updated.

## Comments

* If any other functions that expect compound expressions come to mind, let me know.

* Expressions that fit in a single line remain unchanged:
```wl
  If[!FileExistsQ[tempBuildInfoFile], ReturnFailed["nobuildinfo", tempBuildInfoFile]];
```

## Examples

```wl
  outputWithMetadata = Catch[
    Reap[
      setReplace$wl[initWithMetadata, rulesWithMetadata, stepSpec, vertexIndex, returnOnAbortQ, timeConstraint],
      {$$deletedExpressions, $$events}]
  ,
    $$nonListExpression
  ,
    (Message[caller::nonListExpressions, #]; Return[$Failed]) &
  ];
```

```wl
    result = Catch @ Check[
      If[modifiedEvolution =!= $Failed,
        If[ListQ[property],
          Catch[
            Check[propertyEvaluateWithOptions[#], Throw[$Failed, $propertyMessages]] & /@ property
          ,
            $propertyMessages
          ,
            $Failed &
          ]
        ,
          propertyEvaluateWithOptions @ property
        ] /.
          HoldPattern[WolframModelEvolutionObject[data_Association]] :>
            WolframModelEvolutionObject[Join[data, <|$rules -> rulesSpec|>]]
      ,
        $Failed
      ]
    ,
      $Failed
    ];
```

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/maxitg/setreplace/596)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Does not change functionality, but makes the code better organized or more readable wolfram language Requires Wolfram Language implementation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant