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

refactor: split variables declaration in test #2398

Merged
merged 2 commits into from
Aug 27, 2018
Merged

refactor: split variables declaration in test #2398

merged 2 commits into from
Aug 27, 2018

Conversation

zielint0
Copy link
Contributor

@zielint0 zielint0 commented Aug 20, 2018

I am ready to merge.

@zielint0 zielint0 changed the title refactor: split variables declaration refactor: split variables declaration in test Aug 20, 2018
@@ -294,7 +294,9 @@ public void test() {

private void checkContractCtScanner(CtPackage pack) {
class Counter {
int scan, enter, exit = 0;
int scan;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also explicitly initialize them to 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@monperrus
No. Instead of adding 2 redundant initializers, I removed 1 redundant initializer. So the class is now:

class Counter {
			int scan;
			int enter;
			int exit;
		}

Class' fields have default initialization to 0 (or: "", null) so adding redundant initializer is redundant and confusing. Developer may wonder: why this initializer is added here, if this value is overrided by JVM?

Also it is confusing to trick user by override initializer later:

int x = 0;
... // some non x-activity
x = 2;

I removed such things here: https://github.com/INRIA/spoon/pull/2263/files

Please note that with removing such initializers, I removed other mistakes here. So some analysis paid off ;-)

@surli surli merged commit 9a127a6 into INRIA:master Aug 27, 2018
@monperrus monperrus mentioned this pull request Sep 20, 2018
4 tasks
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.

3 participants