Refactoring of int-key mpc.exe tt file #1088
Conversation
This is a refactoring of int-key mpc.exe tt file.
|
Please don't change original formatting. For example As a basic policy, I don't want to include a lot of logic or multiple lines of code in the template section. |
|
Okay. |
Change the iterator `i` type from `int` to `var`.
|
@neuecc |
There was a problem hiding this comment.
Thanks for contributing, but I'm left wondering what this PR actually does that's useful. "Refactoring" isn't by itself valuable unless it makes some change in the future easier. Can you update your PR description to 'sell' it more? What does it make better/easier to do in the future?
One thing I can see is that you add global:: to several things, which I imagine makes the generated code more resilient. But you have a bunch of other changes (mostly whitespace which I'd like to see reverted). If there's other good stuff here, can you list them so we know what to look for and appreciate while reviewing?
T4 is particularly obtuse, so anything you can describe in your PR description is appreciated.
sandbox/Sandbox/Generated.cs
Outdated
| { | ||
| if (value == null) | ||
| { | ||
| writer.WriteNil(); | ||
| return; | ||
| } | ||
|
|
||
| IFormatterResolver formatterResolver = options.Resolver; | ||
| var formatterResolver = options.Resolver; |
There was a problem hiding this comment.
Why? Please stop mixing irrelevant changes with your PRs. It just makes reviews take longer.
Also, even if you made this style change in a separate PR, I would reject it. var should only be used when the type name already appears elsewhere on the line such that it is obvious.
There was a problem hiding this comment.
Please stop mixing irrelevant changes with your PRs.
I'm sorry. I take your pointing out seriously and will not do it next time.
varshould only be used when the type name already appears elsewhere on the line shuch that it is obvious.
I have a different thought that all generated C# codes must not use using namespace directives.
using namespace directive causes ambiguous type name errors.
var is one good solution to avoid using namespace directives like global::.
var and global:: make the program more resilient.
global::MessagePack.IFormatterResolver formatterResolver = options.Resolver;The above seems the better.
There was a problem hiding this comment.
Oh! A very interesting point. Good insight into the danger of being explicit on typing in a type generated file. Ok, go ahead and use var for IFormatterResolver. But when you can use a keyword (e.g. int) please use that instead of var, as that should still be safe to use.
Revert: DepthStep position Revert: }#> -> } #>
Would you prefer that I merge instead of squash to complete this PR then? |
|
Thank you for the approval. I like |
This is a refactoring of int-key mpc.exe tt file.
This is a subset of #1074.
This PR does not contain any optimization.