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

Replace all Module's and Block's with ModuleScope and Scope #460

Closed
daneelsan opened this issue Oct 14, 2020 · 0 comments · Fixed by #461
Closed

Replace all Module's and Block's with ModuleScope and Scope #460

daneelsan opened this issue Oct 14, 2020 · 0 comments · Fixed by #461
Assignees
Labels
refactor Does not change functionality, but makes the code better organized or more readable wolfram language Requires Wolfram Language implementation

Comments

@daneelsan
Copy link
Collaborator

daneelsan commented Oct 14, 2020

The problem

Benefits of using ModuleScope and Scope (instead of Module and Block):

  • Not necessary to add new variables at the top of the block, i.e. Module[{variables...}, ...] (especially annoying in large functions).
  • Changing variable names is simpler (again because you don't have to rename the names at the top).
  • Merging and splitting functions is easier as it is not necessary to merge and split their argument lists.
  • No performance cost as these functions are (macro) expanded at definition time.

However, as @maxitg said:

... we still want to use Module in pattern rules (e.g., {{a_, b_}} :> Module[{c}, {{a, c}, {c, b}}]). ModuleScope is undocumented so we cannot expose it to users.

Examples

You can use MacroExpand to see what these functions do:

In[] := MacroExpand[foo[] := ModuleScope[
   a = 1;
   b = 2;
   c = 3;
]]

image

In[] := MacroExpand[bar[] := Scope[
   a = 1;
   b = 2;
   c = 3;
]]

image

Comment

It is not simply a matter of replacing all Module's with ModuleScope's. See:

In[] := MacroExpand[{x_, y_} :> ModuleScope[{x, y + 1, z}]]

image

You can use ScopeVariable to "cause particular variables to be localized":

In[] := MacroExpand[{x_, y_} :> ModuleScope[ScopeVariable[z]; {x, y + z}]]

image
But it may not be worth replacing them in these cases.

@daneelsan daneelsan added refactor Does not change functionality, but makes the code better organized or more readable wolfram language Requires Wolfram Language implementation labels Oct 14, 2020
@daneelsan daneelsan self-assigned this Oct 14, 2020
@daneelsan daneelsan mentioned this issue Oct 14, 2020
4 tasks
daneelsan added a commit that referenced this issue Oct 19, 2020
## Changes
* Closes #460.
* Replaces (almost) all `Module`'s with `ModuleScope`'s.
* Adds ```PackageImport["GeneralUtilities`"]``` where `ModuleScope` was used.

<!-- 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/461)
<!-- 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