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

Add extract variable #36

Open
wants to merge 34 commits into
base: master
Choose a base branch
from
Open

Conversation

alquerci
Copy link

@alquerci alquerci commented Sep 27, 2022

Hi,

Firstly, thanks for this great Vim plugin.

Look at the documentation to see the details.

Tested on VIM 8.1

In order to add proof that the code work as it should, I choose to use https://github.com/junegunn/vader.vim as test runner.

Why we need this ?

Extract Variable is part of the must-have tools of a programmer as this refactoring is done too many times.

If Extract Method is the most important of refactorings, Extract Variable is its ready assistant. It turns out that in order to extract methods, you often must extract variables first.

@Test
public void allOnes() throws Exception {
    for (int i=0; i<20; i++)
        g.roll(1);
    assertEquals(20, g.score());
}

And we ended up with this:

private void rollMany(int n, int pins) {
    for (int i = 0; i < n; i++) {
        g.roll(pins);
    }
}

@Test
public void allOnes() throws Exception {
    rollMany(20, 1);
    assertEquals(20, g.score());
}

The sequence of refactorings was as follows:

  1. Extract Variable: The 1 in g.roll(1) was extracted into a variable named pins.
  2. Extract Variable: The 20 in assertEquals(20, g.score()); was extracted into a variable named n. The two variables were moved above the for loop.
  3. Extract Method: The for loop was extracted into the rollMany function. The names of the variables became the names of the arguments.
  4. Inline: The two variables were inlined. They had served their purpose and were no longer needed.

Another common usage for Extract Variable is to create an explanatory variable.

if (employee.age > 60 && employee.salary > 150000)
    ScheduleForEarlyRetirement(employee);

This might read better with an explanatory variable:

boolean isEligibleForEarlyRetirement = employee.age > 60 && employee.salary > 150000;

if (isEligibleForEarlyRetirement)
    ScheduleForEarlyRetirement(employee);

From "Clean Craftsmanship: Disciplines, Standards, and Ethics (Robert C. Martin Series) (English Edition)" written by Robert C. Martin

@zaxxx
Copy link

zaxxx commented Dec 10, 2023

hi man @alquerci , huge thank you for this, so glad I found it!

apparently it's not going to get merged, so if you don't mind, I'm just gonna use your fork in my packer config 😅

I've been using vim.lsp.buf.code_action refactor.extract.expression, but it doesn't have the courtesy of asking me for a variable's name and it's asynchronous, so I can't easily chain it with PhpRenameLocalVariable... a workaround is to use something like vim.wait(2000) between those commands... ugh.. but your work is exactly what I need and will be a huge time saver in the long run. So, thanks again!

@alquerci
Copy link
Author

Hello @zaxxx,

I am glad that my work can help you. It helps me a lot in daily work bases.

Since one year, a lot of stuff has been done. All of them has been done regarding issues or needs I caught.

Some refactorings are never used and others are overused.

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

Successfully merging this pull request may close these issues.

2 participants