Refactor transformations logic

[andrew-boyarshin]

• Add ITransformationSet interface and noop implementation
• Add BasicReadTransformationSet to be used as default (enough for analyzers)
• Add ChainTransformationSet that allows chaining multiple sets
• Add Vs15TransformationSet that contains all implemented transformations
• Add TargetTransformationExecutionMoment enum (Normal/Early/Late)
• Add ITransformationWithDependencies and ITransformationWithTargetMoment
• Refactor ProjectConverter.TransformationsToApply() to do topological sort (ordering) on a transformation graph (to be precise - reverse graph) by stages (TargetTransformationExecutionMoment)
• Fix NuGetPackageTransformation to have NuGet casing (I certainly remember it being fixed some time ago)

No more manual ordering issues (like during development of #177), everything is automated.

[andrew-boyarshin]

I've addressed ProjectConverter constructor feedback and pushed amended commit. Other notes are mostly on overengineering (although I believe the code that does things in O(n) instead of O(2n) and still looks pretty readable (6 codelines instead of 2 long lines) should not cause such feedback).

[hvanbakel]

Don't take it the wrong way, that's why I approved the changes. The things I brought up are not structural issues, I always feel like the discussions are very informative as to why certain things were solved in a specific way. Everyone is biased by their normal code practices which makes opensource refreshing

[andrew-boyarshin]

I'm not taking it the wrong way, but I do wonder why people think Linq is the ultimate answer for everything (not to mention actual Linq that is statements starting with from, limited as ever). Trivial things (filter and call method on found items) can indeed be simplified with Linq, that's natural. But when people try to express more complex logic in Linq they tend to omit the additional code readability drop and performance overhead they introduce by doing that. Linq doesn't simply solve the issue, it provides an abstraction level for any IEnumerable<T>, but developer must still remember that the inner costs are hidden, not removed. In my projects I have ReSharper suggest a number of cases that can be simplified, including replacement with Linq. But when I apply the quick-fix, I understand that I can no longer perceive what this code is for. Furthermore, with Linq it gets increasingly harder to introduce changes, since you can't deviate from the pattern Linq enforces. And yeah, it's hard to disagree that everyone is biased according to their own set of experiences.

[mungojam]

If the set of transformations no longer has a default that will make it a bit trickier to use the nuget package. It's probably fine if we can add something to the readme showing how to use the package for a default run and for one with an additional custom transform

[mungojam]

The linq one is definitely style and preference. If you ever get into F#, you might see where I'm coming from. I don't use it everywhere but default to it until I need to improve performance.

I generally find it much easier to establish if code is right for example I have a gaurantee that a Select will give me a 1 to 1 mapping and an Any won't be editing anything but just checking each item.

It is a different dialect, familiar to those with functional programming experience as they are all standard opeationsm. Over time the performance differences will be hopefully removed by better compilers and JITer

[andrew-boyarshin]

@mungojam we already require ILogger and don't exactly specify how to obtain an instance of one. Adding another argument that is clearly used in Program the correct way depending on the needs doesn't seem that bad. I've modified it to be mandatory according to @hvanbakel's suggestion and I see the point in that. There are 3 implementations now: Noop, BasicRead and Vs15, each has its uses and it is better for NuGet package users if they can precisely specify what set to use (or create their own, by chaining with one of the existing or writing from scratch).

[mungojam]

Sure, but consumers shouldn't need to look in source control for how to use it. You're right about ILogger and it was a bit annoying. The resolution for me is a better readme that gives default usage. I can try putting something together at the weekend

[andrew-boyarshin]

@mungojam yeah, I know that was a good attempt at bringing functional-style into C#, and I like some elements of that. I've written this code in imperative fashion. There are pros and cons to that. Still, we can't write code that satisfies everyone. If there are no further notes or major issues I believe this can be merged. P.S. I am working on next PR now, related to CLI.

[andrew-boyarshin]

Ok, I've taken liberty and merged this PR since everything discussed here is non-critical, highly opinionated, or fixed (and I'd rather next PR be based on master instead of my branch).

[hvanbakel]

Nice @andrew-boyarshin, there’s a reason I made both of you collaborators, I don’t always have the time to merge/review quickly enough.

The discussion on using linq or not is mostly opnions I guess. However, I would keep this in mind

"Programs should be written for people to read, and only incidentally for machines to execute." -- Structure and Interpretation of Computer Programs

Linq does make code more readable and virtually everyone who writes C# knows how to read it. There’s absolutely overhead in there but that would be something to address in hotspots in your code after detailed analysis of where those hotspots are. Especially on big(ger) teams this becomes paramount because you can more easily grow the team to work on the non performance critical parts.

And to be completely clear, I wouldn’t be the one writing the performant code, I would need to google what stackalloc does but that’s fine (and you haven’t used that yet).