Backwards compatibility

[jasonnator]

Is there an interest in providing backwards compatibility for less than C# 7? If so, I have a PR I'd like to submit with only those changes required to support < C# 7.

[Dirkster99]

This sounds like a good idea - Can you describe in more detail why you think staying below C# 7 is a good idea?

Is there a setting in VS that disables >= C# 7 or is this bound to a certain version of .Net Framework?

I have not given this a lot of thought, yet, but would be intersted to review and understand your PR better.

Thanks Dirk

[jasonnator]

Doing this from my phone so here goes.

The main issue is the way the if statements are written with the "is" check then instantiating a variable with an implied "as".

I believe there were about 220 instances that I changed with only a few requiring additional if statements to ensure proper logic flow. Also, the double "??" wasn't compatible which surprised me. Again, simple if else rewrite.

There was also a build error caused by the post build command hinting at ensuring an assembly was in the path. That could easily be moved to the Readme so Edit complies first time out of the box. Although it was a simple fix, CSC errors can be daunting to developers who may not be familiar with how to read the nuggets within the error. That post build command caused a cascade of CSC errors. IMO, the bang for buck would be to make a note in the Readmeband ensure fist time compile.

The only downside I see is there aren't null checks for the local scope variable casted with as. I'm not certain C#7 implies a null reference check (it very well may). The dispatcher unhandled exception will catch them as a last resort.

Example
if (this.mActiveDocument is MiniUmlViewModel vm)
{
return vm.DocumentMiniUML as MiniUML.Model.ViewModels.Document.AbstractDocumentViewModel;
}

The implied instantiation of vm is what gives < C#7 heartache.

Instead, it can be easily rewritten to:
if (this.mActiveDocument is MiniUmlViewModel)
{
MiniUmlViewModel vm = this.mActiveDocument as MiniUmlViewModel;

// would be best to wrap a null if check here to ensure as was able to cast

                return vm.DocumentMiniUML as MiniUML.Model.ViewModels.Document.AbstractDocumentViewModel;
            }

[jasonnator]

Re staying below C#7:

VS 2015/2013 are still very widely used as well as C# 6 and even lower.

The syntactic sugar is nice but could turn away some intermediate level developers from using this entire repo for the excellent mvvm showcase it is.

[jasonnator]

I believe .NET 4.7 introduces C#7 so anything below that would be incompatible. Me personally, I primarily used 4.6.2 and 4.5 so yeah, it's a bit selfish.

[Dirkster99]

Hi, your proposals sound interesting - I am also focused on 4.6.2 and 4.5 so later versions are not intresting, yet.

I also looked at making re-builds more stable (by providing nugets) and other methods - unfortunately, I am not a good build master :-( and things can get quit complecated with MEF:

• PostBuildEvent
• MEF App.config

I got these hints for improving the project but was unable to implement them, yet, but it would be great if you could contribute something towards that end. Its probably best if you could create 2 PRs - one for the <C#7 issue and one for improving the build (if you see that). Thanks Dirk

[jasonnator]

Dirk,

I believe Msbuild is your savior for what you describe. I'll see if I can help on that but will go ahead and submit this one.

Thanks,

Jason