[Refactor] Added Prefer var everywhere#3216
[Refactor] Added Prefer var everywhere#3216cschuchardt88 wants to merge 3 commits intoneo-project:masterfrom
Refactor] Added Prefer var everywhere#3216Conversation
Refactor] Changed dotnet types to var variable typeRefactor] Prefer var everywhere
AnnaShaleva
left a comment
There was a problem hiding this comment.
@neo-project/core, do we really need this change? This is a large and old codebase, it has a lot of contributors, and it's quite understandable that different code styles are used here. I'm against of moving the whole codebase to some single standard in this way, because it's not a vital necessity. Also, it updates the git history of some lines, it'll distract while searching for some protocol changes in git.
We may write the new code following some new standards, but I don't think we should touch the old code without a good reason.
The last argument (a personal one, so probably might not be taken into account): I prefer explicit types, it's easier to review and understand the code with them.
|
@AnnaShaleva we already agreed on this long ago. I think around the time of the mono repo move. No one had time to fix the repo, because we were porting libraries to |
I am in agreement with you @AnnaShaleva, if it is just style for now. |
|
We already agreed with this change and the rules is already an enabled; so I'm enforcing it now and updated the code. See #3023 (comment) for discussion Plus this |
Refactor] Prefer var everywhereRefactor] Added Prefer var everywhere
|
Agree with @AnnaShaleva and @vncoelho |
|
This is has nothing to do with style. This fixes more issues than not having it. Please read old discussion to refresh your memory of why we need it. And again this rule is already enabled in the current repo. I'm just enabling |
|
In my opinion, this can be a very damaging change, since it's not obvious which classes are returned in specific cases, and this will increase confusion with no extra benefits. From what I understood, there are three rules:
First, is for cases such as: Second, is for: Third is for the rest. Particularly, I find specially damaging this specific change: since this permanently hides the array type, and can even change the interpretation of the developer regarding stack vs heap type. So, for me, it's completely insane rule. From the three rules, I only agree with the second, and I find it beneficial for us, so I won't disagree for enabling it only. |
|
@igormcoelho That is not how it works. In the Becomes defined to a class:Can be changed to a different type:With that said. It shows that you can't easily be tricked or forgotten what it was; just hover over it.
|
|
Improved code readability: NO, you need to move your mouse, and wait to know the type, that's not improve the readability. Try to see the type in github, maybe you are reading the code in github. |
|
Use var when you like, not use when you dont like, this is not a technical problem, totally personal preference. Should not be forced. general type and automatic type inference are normal trend, wont cause any readability issue, otherwise go, rust, js, python and verious new languages should go die. Thus, i wont vote banning it, nor will i vote use it everywhere. |
vncoelho
left a comment
There was a problem hiding this comment.
At least @igormcoelho comments should be attended.
His point 2 should be merged in another PR.
The other changes I am against now.
Something like that,@Jim8y . |
|
no resolution |
@cschuchardt88 not everyone uses IDE, as many discussions happen here, directly on GitHub, that cannot deduce any type. So it will overcomplicate discussions and reviews.
I did not say that. I mentioned that |
|
|



Change Log
vareverywhereChecklist: