[Refactor] Added Prefer var everywhere

[cschuchardt88]

Change Log

• Prefer var everywhere

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules

[AnnaShaleva]

@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.

[cschuchardt88]

@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 dotnet standard.

[vncoelho]

@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.

I am in agreement with you @AnnaShaleva, if it is just style for now.
In this sense, I also prefer to not move this forward.

[cschuchardt88]

@vncoelho @AnnaShaleva

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 var is the way we should be writing code anyways.

@shargon @Jim8y Input please

[shargon]

Agree with @AnnaShaleva and @vncoelho

[cschuchardt88]

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 dotnet format to check for it now.

[igormcoelho]

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:

1. csharp_style_var_for_built_in_types = true:warning
2. csharp_style_var_when_type_is_apparent = true:warning
3. csharp_style_var_elsewhere = true:warning

First, is for cases such as:

int x = 10;
var x = 10;

Second, is for:

Transaction tx = new Transaction

Third is for the rest.

Particularly, I find specially damaging this specific change:

Signer[] signers = Array.Empty<Signer>();
var signers = Array.Empty<Signer>();

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.

[cschuchardt88]

@igormcoelho That is not how it works. In the IDE it is typed to the 1st instance of the of whatever class you set it to. Its an ide thing; at compile time it typed to that instance of the class I talked about. So it can't equal anything else besides the its inheritances. Its not setting it to a object. If that's what you all are thinking.

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.

@Jim8y
And another issue is that we may not really remember or even know what the type is, but we can still write code, IDE will tell us the type later. If you ban var, every time we want to define a variable, we must know the exact type before we can go on with the processing logic, types can be:

Stack
Dictionary<ILocalSymbol, byte>
Dictionary<ILabelSymbol, JumpTarget>
Dictionary<IParameterSymbol, byte>

1. Improved code readability: By using var, developers can avoid having to explicitly specify the data type of a variable, which can make the code more readable and easier to understand. This is especially useful when working with complex data types or generic collections.

2. Reduced code verbosity: When working with long or complex data types, the use of var can help reduce the amount of code needed to declare a variable. This can make the code more concise and easier to maintain.

3. Better support for refactoring: When the data type of a variable is inferred using var, it becomes more resilient to changes in the codebase. For example, if the data type of a variable is changed, the compiler will automatically update all the references to that variable, reducing the risk of breaking the code.

4. Improved performance: In some cases, the use of var can lead to improved performance, as the compiler can generate more efficient code when the data type of a variable is inferred.

5. Consistency with other modern languages: Many modern programming languages such as Python, JavaScript, and Go already use type inference, and it's becoming a common practice among developers. Using var in C# can help developers to have a more consistent experience across different languages.

[shargon]

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.
Reduced code verbosity: Ok, i can be agree with this point.
Better support for refactoring: IDE helps here, and change the type also.
Improved performance: No, it's the same.
Consistency with other modern languages: It's not a problem.

[Jim8y]

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. var is a C# language feature, should not be banned, should not be enforced.

Thus, i wont vote banning it, nor will i vote use it everywhere.

[vncoelho]

At least @igormcoelho comments should be attended.
His point 2 should be merged in another PR.
The other changes I am against now.

[vncoelho]

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. var is a C# language feature, should not be banned, should not be enforced.

Thus, i wont vote banning it, nor will i vote use it everywhere.

Something like that,@Jim8y .
In C++,one of the main use cases of auto was for "range-based for loop"

[cschuchardt88]

no resolution

[igormcoelho]

@igormcoelho That is not how it works. In the IDE it is typed to the 1st instance of the of whatever class you set it to. Its an ide thing; at compile time it typed to that instance of the class I talked about.

@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.

So it can't equal anything else besides the its inheritances. Its not setting it to a object. If that's what you all are thinking.

I did not say that. I mentioned that var explicitly hides the type, regardless of being an array or not. This is not an acceptable behavior in other languages, such as C++, where auto (equivalent of var) does not force hiding modifiers for pointers (*), arrays ([]), references &, rvalue references && and so on. But at least on C++ all of these (except from references) have value-type semantics, differently from C#.

[Jim8y]

var explicitly hides the type, regardless of being an array or not. is a ridiculous reason. Its 2024, modern languages all support var- like type inference, I dont see any problem when I use them. Only if you still use text editor to write code, otherwise there should be no problem of using var at all. And please stop considering C++ as an example here, it is bloated and burdened with its long history. ITS A TOTAL PERSONAL PREFERENCE.