MathHelper.cs issues #21

Closed
birbilis opened this Issue May 25, 2015 · 23 comments

Projects

None yet

2 participants

@birbilis

I see MathHelper.cs has some Russian comments in it, can you change those to English?

Also, I see very similar versions of it at GraphX.METRO.Controls and at GraphX.WPF.Controls that I can easily merge into one using some conditional compilation block near the top (the rest is very similar code as I see with WinMerge).

The question is where would the merged file be put? Should it go to GraphX.PCL.Common or to GraphX.PCL.Logic? I only see a Helpers/ThreadingHelper.cs at the 2nd one and an empty Helpers folder at the 1st one. Also I'd need to change the namespace to GraphX.PCL.Common.Helpers I guess instead of GraphX.WPF.Controls and GraphX.METRO.Controls that the two files have now

BTW, why use different namespaces in GraphX? You will never have WPF and METRO library linked together in the same project, so why not use the same namespace so that client code that uses the library can easily be ported (or shared via linked files) between WPF and METRO apps (and the coming UWA ones)? I've been using this practice in ClipFlair Studio and other projects and I'm very happy with code reuse (without even maintaining separate files, just have multiple projects for different platforms reference the same sources via linked files) like that.

...in my projects I even set the target assemblies to not have a platform in their name so that I can reuse XAML files (via linked files again) between WPF and Silverlight (else I'd need to have different XAML cause of XAML namespace definitions that needed assembly name). Metro unfortunately added the "using" syntax for XAML namespaces and MS uses that with UWA apps too and I think they don't support the older syntax at all which is very sad since they force you use separate XAML files in such case (given that there is no XAML include directive to include the rest of the common XAML between platforms). Can check the pattern I use with linked files at http://clipflair.codeplex.com/SourceControl/latest#Client/ZUI/ImageButtons/ for example (see the folders)

@panthernet
Owner

I see MathHelper.cs has some Russian comments in it, can you change those to English?

Yes i can, must had missed them

Also, I see very similar versions of it at GraphX.METRO.Controls and at GraphX.WPF.Controls that I can easily merge into one using some conditional compilation block near the top

The problem with this file is that it uses (or extends) classes like Point that are not PCL compatible so they can't be placed in PCL. Can be placed in WPF project and be linked to METRO though (using conditional directives).

BTW, why use different namespaces in GraphX?

Honestly, i haven't tried to implement cross-platform apps so some things can be out of my sight. The one namespace sounds nice :) Also i've completely forgot about file linking benefits, it would be nice to reinspect WPF and METRO for files to combine.

In summary, first of all i'll try to implement single namespace for WPF and METRO. Then i can take a look at these projects files to see if i can merge some into one.

@panthernet
Owner

PS: Also i'll make MathHelper files more similar for easier merge. I'll try to upload changes tomorrow. I have a lot of job to do so can't promise anything.

@panthernet
Owner

Changed namespace to GraphX.Controls across WPF and METRO libs. I suppose i had to change assembly name to that name too but i like the platform name reflected in file name. Still not sure.

@birbilis

Can use same assembly name and file too because it helps with classic XAML namespace declarations (not the METRO ones that do "using:"

That way you can reuse XAML files (via linked files or shared project [there is VS extension for such])

Not sure if NuGet packaging has issue, probably not though

@birbilis

At https://msdn.microsoft.com/en-us/library/ms747086(v=vs.110).aspx it says:

assembly= The assembly that contains some or all of the referenced CLR namespace. This value is typically just the name of the assembly, not the path, and does not include the extension (such as .dll or .exe). The path to that assembly must be established as a project reference in the project file that contains the XAML you are trying to map. In order to incorporate versioning and strong-name signing, the assembly value can be a string as defined by AssemblyName, rather than the simple string name.

so most probably XAML namespace declarations search for the assembly name, don't care of filenames, so to be able to reuse XAML files via file linking or shared projects (between WPF and Silverlight that use the same XAML namespace syntax that is), I guess you can just use the same assembly name and different filenames to show the platform (like xx for the assembly name field at the project properties/build tab and xx.WPF.dll or xx_WPF.dll for the output filename)

I think I was having some issues though with this practice (don't remember for sure) and I decided to use same value for BOTH the assembly name AND the filename eventually for all versions

BTW, for METRO (Win8 store) and UWA apps there is new syntax:

•Instead of using a clr-namespace:/assembly= qualifier set for code-to-XAML namespace references, you use the using: qualifier. XAML namespaces no longer reference specific assemblies; all assembly qualification and inclusion is handled by the application model and how you compose your app package.

but haven't tried it yet to see if the older syntax is also supported (fear it isn't)

ALSO, see http://www.codeproject.com/Articles/111911/A-Guide-to-Cleaner-XAML-with-Custom-Namespaces-and - IT IS MAYBE BETTER TO DEFINE A URL FOR THE NAMESPACE (YOU DO IT IN ASSEMBLYINFO.CS FILE). If this (using a URL) is still supported at METRO and UWA XAML, then it is much better since you'll be able to reuse XAML files (via shared projects or linked files) between WPF, Silverlight, Win8 Store Apps (WinRT) and UWA (Win10) apps. Have to look a bit into it though

@birbilis

regarding the XmlnsDefinition I mention above (for AssemblyInfo.cs), an article from 2012 says WinRT doesn't support neigther 1:1, nor 1:n (another nice feature of Silverlight/WPF) namespace mapping using such URLs (it doesn't have XmlnsDefinition attribute). Will check in case Microsoft added support for that, or what happens if one defines the XmlnsDefinition attribute themselves (if it is just ignored then in WinRT)

The 1:n-Namespace-Mapping with XmlnsDefinition-Attribute

The Namespace-Mappings above have been 1:1-Mappings. One XML-Namespace was mapped to one Silverlight/WinRT-Namespace.

In Silverlight-XAML you can create a 1:n-Namespace-Mapping by placing the XmlnsDefinitionAttribute (Namespace: System.Windows.Markup) on your assemblies like this:
[assembly: XmlnsDefinition(
"http://thomasclaudiushuber.com/","UIRocker.Special")]
[assembly: XmlnsDefinition(
"http://thomasclaudiushuber.com/", "UIRocker.Mvvm")]

Classes out of the Namespaces UIRocker.Special and UIRocker.Mvvm can be used in XAML with one alias by making a 1:n-Namespace-Mapping like this:
xmlns:rocker="http://thomasclaudiushuber.com/"

Unfortunately in WinRT-XAML there is no possibility for a 1:n-Mapping. The Namespace Windows.UI.Xaml.Markup contains a XmlnsDefinition, but it’s not an attribute, it’s a struct and therefore not usable.

Especially when you create a library with many Namespaces in it, it’s great to use just one alias for the library in XAML. Maybe the next Version of WinRT will contain such a mapping. By the way, first versions of Silverlight also didn’t support 1:n-Namespace-Mappings.

@birbilis

A discussion at https://social.msdn.microsoft.com/Forums/windowsapps/en-US/026baea0-6324-46ee-956a-72dbb4c90ca1/xmlnsdefinition-replacement-in-winrt?prof=required says:

"...there is no analogous attribute.
You might be able to provide similar data in a custom IXamlMetadataProvider implementation, but since that is automatically generated for you I'm not sure it can be overridden."

there is some more info related to IXamlMetadataProvider here: http://stackoverflow.com/questions/12747591/possible-to-use-a-uri-as-an-xml-namespace-in-winrt-xaml

also found some encouraging info at:
http://www.sharpgis.net/post/2013/05/30/Decoding-the-IXamlMetadataProvider-interface-Part-1

"So the auto-generated code automatically detected that my custom library has a second metadata provider embedded, and injects it into this list as well as the auto-generated one. So it looks like we should be able to provide our own implementations, which I’ll get back to later."

This also has useful info: http://www.jaylee.org/post/2012/03/07/Xaml-integration-with-WinRT-and-the-IXamlMetadataProvider-interface.aspx

@panthernet
Owner

but haven't tried it yet to see if the older syntax is also supported (fear it isn't)

I've tried to use it but it seems it's not supported :(

IT IS MAYBE BETTER TO DEFINE A URL FOR THE NAMESPACE (YOU DO IT IN ASSEMBLYINFO.CS FILE)

That's super cool feature i've also forgot about :) I knew of it but actualy haven't used yet. Thanks, i'll take a look and try to adapt it for GraphX! I see there are some problems with WinRT so i'll check it also.

Many thanks for pointing me such good coding practices! :)

@panthernet
Owner

I've failed to implement IXamlMetadataProvider for METRO. It requires fields to be implemented that are not implemented in all examples i've found and much more time for an investigations. I might look at it later or you can play with it if you want. I leave sources in GraphX.METRO.Controls\Models\XamlTypes with commented XamlMetadataProvider to not trigger it.

I've also changed assembly name to the identical GraphX.Controls and added "http://schemas.panthernet.ru/graphx/" XAML namespace for WPF.

@panthernet
Owner

Also, please inform me if you want to do some cleaning by referencing WPF files to METRO instead of identical duplicates cause i plan to do it too. So there will be no overhead. Thanks!

@birbilis

Will not have good network till the weekend so feel free to work on it.

There is a Compare files extension for Visual Studio that can work with WinMerge, but there is also an amazing compare tool for free as VS extension that shows VS editing panes side by side with intellisense etc. AMAZING producivity when merging separate WPF and Silverlight implementations of a library with that one is my experience. Will need to checkout the name of that exrension, maybe was CodeCompare

I love doing refactoring and have some stuff already to submit but I'm confused with GitHub desktop's Pull request dialog. It tries to make pulls for master, not PCL branch

@panthernet
Owner

Ok, got it, thanks! I'll check that extension. I haven't submitted anything to other projects on GitHub so i'm afraid i can't help with this one :( But surely there must be some option to select correct branch. I've used SmartGit before VS started to support git repos natively.

@birbilis

Haven't tried vs native git support, that might be better option than the github desktop app, although that one looks very nice

@panthernet
Owner

I haven't thought that would be so exciting! I'm not only able to merge substantial amount of code but also able to see and implement additional performance/feature tweaks and fix some bugs found in the process. For example i've merged different edge pointer controls into universal one and notably increased edge drawing performance. Currently i'm somewhere past the middle of the work that should be done and slowly move to the end :) There are also some issues that slows me done and had to be solved but that's makes the task interesting :)

@birbilis
birbilis commented Jun 3, 2015

looking at the folders/projects/libraries/namespaces naming, I think it would be more appropriate to add the platform at the end of the name, say GraphX.somePart.PCL, GraphX.somePart.UWA (=Universal Windows Application model [aka Win10]) etc. Not sure how easy it will be though for contributors to merge pending changes via GitHub if you do such drastic changes (I'm still struggling with Git myself, prefer Mercurial)

@birbilis
birbilis commented Jun 5, 2015

Speaking of moving the platform to the end of the name (e.g. GraphX.Controls.WPF, GraphX.Controls.SL5 etc.), to do it on the folder names (apart from doing at the source code for packages, target assemblies etc.), I think one has to use some Version Control command to "move" (Git should have something like that) the files to the new folder, else other contributors may have issue merging their changes.

Despite the trouble to do it, I think it is better for the long run. Also, all GraphX.SomePart.* subfolders could then be grouped in a GraphX.SomePart folder as subfolders where a Source or Common subfolder would also exists that has all the common code those platform-specific versions of GraphX.SomePart share (via linked files to ..\Common\SomeFile, ..\Common\SomeFolder\SomeFile and ..\Common\SomeOtherFolder\SomeFile etc.)

This is the scheme I've been using (and I'm very satisfied with) at http://clipflair.codeplex.com and other projects (e.g. at the AmnesiaOfWho game [http://facebook.com/AmnesiaOfWho] that has separate versions for SL5, WP7 and WP8 and WPF version on the works, plus WRT (WinRT) and UWA [Universal Windows App] too coming in the near future, with all code and most XAML shared via linked files and UserControl[s])

@panthernet
Owner

Hi, i can't see much problems in graphX with these. Platform specific GraphX versions are contained each in own project and separate folder reflecting platform name (excl. WPF version folder name for now). All platform specific projects share single GraphX.Controls namespace now based on your recommendations and i share cross-platform files as links now.

PCL stands aside as unified library :)

@birbilis
birbilis commented Jun 5, 2015

I meant I'd expect GraphX.Common folder with GraphX.Common.PCL in it and GraphX.Common.WP7 etc. versions for example also in that folder since PCL is usually set at WP8 level. This is just an example.

What I'm saying is that the platform name is the last part of the specialization chain, so logically GraphX comes first then say comes Controls then comes WPF, SL5, WP8 etc. in the folder name.

Also would have 3 parent folders Controls, Common and Logic. The last two would just contain the respective .PCL subfolder for now, but I can contribute subfolders for specific platforms too, esp for those the common PCL profile doesn't cover. Source would be at Controls\Source, Common\Source, Logic\Source and respective platform specific projects (even the pcl projects) would use linked files

aka

GraphX (contains GraphX.sln)

--\Controls
----\Source
----\GraphX.Controls.WP7
----\GraphX.Controls.WP8
----\GraphX.Controls.SL5 (contains GraphX.Controls.SL5.csproj)
----\GraphX.Controls.WPF (contains GraphX.Controls.WPF.csproj)
----\GraphX.Controls.WIN8
----\GraphX.Controls.UWA
---- ... (more platform specific versions)

--\Common
----\Source
----\GraphX.Common.PCL
---- ... (platform specific versions, esp. those not covered by the settings chosen at the PCL)

--\Logic
----\Source
----\GraphX.Logic.PCL
---- ... (platform specific versions, esp. those not covered by the settings chosen at the PCL)

The same structure would also be used at examples to cover the potential of porting some of them to more platforms:

--\Examples
----\SomeExample
------\Source
------\SomeExample.WPF
------\SomeExample.SL5
----\SomeOtherExample
------\Source
------\SomeOtherExample.WIN8
------\SomeOtherExample.UWA
etc.

Of course common code at each folder is at the Source subfolder of that folder, shared using linked files (e.g. at \Examples\SomeExample\Source) and platform specific code is inside the respective projects (e.g. at \Examples\SomeExample\SomeExample.WPF)

Similarly the GraphX.sln would contain solution folders "Controls", "Common" and "Examples", though it could also contain separate solution (virtual) folders per platform that have each one of them "Controls", "Common" and "Examples" in them. That is the solution's folder structure is organized per-platform in such a case. This is mostly useful if you want to focus on a specific platform each time when developing. However since the solution folders are virtual, one could even go as far as having two solutions, one with the same structure as the filesystem folders I suggest and one with a per-platform/target structure.

I follow the per-platform virtual solution folders style at ClipFlair.sln in http://clipflair.codeplex.com, while the real folders are structured as I describe above (where each module has its own physical subfolders for the various platforms). In fact some module subfolders there (say Client\ZUI\ColorChooser) contain their own extra solution file when I want to be able to focus just on a certain module. That solution just includes the respective ColorChooser.WPF, ColorChooser.SL5 etc. subprojects from respective subfolders). Such solutions also contain virtual WPF, Silverlight etc. subfolders that has as children apart from the respective platform-specific project (say ColorChooser.WPF) any other platform-specific projects needed (e.g. ....\Helpers\Utils\Utils.WPF\Utils.WPF.csproj etc.) by that module to compile

@birbilis
birbilis commented Jun 5, 2015

btw, regarding PCL useful resources are:

Speaking of Xamarin, adding support for that too could follow the same pattern as described above (PCL where possible and platform-specific versions for .XamarinIOS, .XamarinAndroid etc. where needed)

@panthernet
Owner

Thanks for the analyzers, i'll play with them a bit later. Regarding structural changes, honestly, i don't have much time now and no will either to do them though they are seemed quite logical to me. Currently the file structure is a bit wacky if you look at it from the explorer, but in VS it is operable and clean. I think i'll return to that question in a while, may be i should make completely new upload and use master branch with the new file structure... hm...

@birbilis

btw, since I saw at some reply of yours that you have some build task to rename the .DLLs, I see NuGet is suggesting to output the DLLs with same filename in different subfolders:

https://docs.nuget.org/create/enforced-package-conventions


When NuGet installs an assembly from a package, it checks the target .NET Framework version of the project you are adding the package to. NuGet then selects the correct version of the assembly in the package by selecting the correct subfolder within the lib folder.

To enable NuGet to do this, you use the following naming convention to indicate which assemblies go with which framework versions:
lib{framework name}{version}

The following example shows a folder structure that supports four versions of a library:

\lib
  \net11
     \MyAssembly.dll
  \net20
     \MyAssembly.dll
  \net40
     \MyAssembly.dll
  \sl40
     \MyAssembly.dll

at http://blog.xamarin.com/how-to-update-nuget-packages-for-64-bit/ Xamarin used different DLLs in the subfolders though as seen at http://blog.xamarin.com/wp-content/uploads/2014/12/NuGetUnifiedLibDirectories.png

guess if you're now using a URL for XAML namespaces, you can choose either approach without problem of reusing XAML code.

@panthernet
Owner

This build task is cosmetic, not sure if i will leave it there as it is now. While messing with QuickgraphPCL NuGet i've learned how true NuGet package should be organized :) It is similar to what you've wrote up there. For Quickgraph i had two subfolders, one for net40 and other for all the others plarforms portable-win+net4+sl5+wp8+win8+wpa81+MonoAndroid10+MonoTouch10+Xamarin.iOS10. So the next GraphX release should have net40 and win81 subfolders.

@birbilis

btw, that article I mentioned at #24 (http://oren.codes/2015/06/16/demystifying-pcls-net-core-dnx-and-uwp-redux) says they're adding a platform agnostic identifier too (named "dotnet") that will mean "see the platforms my dependencies support" - guess that goes down the tree of those dependencies recursively). Problem is I guess it will take some time till Microsoft and others re-release their packages to turn on that one

that article also mentions the Bait & Switch technique (http://log.paulbetts.org/the-bait-and-switch-pcl-trick/) that allows you to make PCLs that are empty (contain just definitions I think [contract]) and fallback to platform-specific implementations automatically without the caller noticing it

@panthernet panthernet closed this Jun 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment