-1
\$\begingroup\$

Intro: Before showing the actual code for review, let me show you an ExampleClass to create a shim for.

using System.Windows;
public class ExampleClass
{
    public void ExampleMethod(string line) { MessageBox.Show(line); }
}

The target API of ExampleClass had been .net framework, before some new requirements came to make it multitargeted (both .net framework and .net core 2.0).

One solution is to create a Shim for ExampleClass. This Shim should have ExampleMethod(..) implemented for two APIs. The shim itself should be working on both APIs.


Code for Review, my first approach to shimming:

Base Shim:

public class Shim
{
    public object Obj { get; private set; }
    public Shim(object defaultObject)
    {
        Obj = defaultObject;
    }
}   
//it is possible to create 
//two platform specific counterpart classes for ExampleClass
//but i don't like this idea because it implies additional work

My multitargeted Shim for ExampleClass:

#if NETFULL
using System.Windows;
#endif
namespace PortabilityLibrary.Shims
{
    public class ExampleClassShim: Shim
    {
        public ExampleClassShim(object obj):base(obj) {   }

        public void ExampleMethod(string line)
        {
#if NETFULL
            if (Obj is ExampleClass)
                ((ExampleClass)Obj).ExampleMethod(line);
            else
                throw new Exception();
#elif NETCORE
            if (Obj is NetCoreExampleClass)
                ((NetCoreExampleClass)Obj).ExampleMethod(line);
            else
                throw new Exception();
#endif
        }
    }
}

The following is no more for review, it could help to better understand the issue:

The way I could use the code from a .net framework assembly:

public class ExampleCode
{
    public static void Do()
    {
        var x = new ExampleClassShim(new ExampleClass());
        x.ExampleMethod("For example...");
    }
}

My sdk-style .csproj file to make clear about NETFULL and NETCORE:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup><TargetFrameworks>netstandard2.0;netcoreapp2.0;net461</TargetFrameworks></PropertyGroup>

  <PropertyGroup Condition=" '$(TargetFramework)' == 'netcoreapp2.0' OR '$(TargetFramework)' == 'netstandard2.0'">
    <DefineConstants>NETCORE;</DefineConstants></PropertyGroup>

  <PropertyGroup Condition=" '$(TargetFramework)' == 'net461'">
    <DefineConstants>NETFULL;</DefineConstants></PropertyGroup>
</Project>

Ok, just in case, a little bit more explanation:

NetCoreExampleClass:

public class NetCoreExampleClass
{
    public void ExampleMethod(string line) { Console.WriteLine(line); }
}

NetCoreExampleCode, the way I could use the code from a .net core 2.0 assembly:

public class NetCoreExampleCode
{
    public static void Do()
    {
        var x = new ExampleClassShim(new NetCoreExampleClass());
        x.ExampleMethod("For example...");
    }
}
\$\endgroup\$
  • \$\begingroup\$ Why Shim with conditional compilation (#if NETFULL) and not conditional compilation for the real implementation? \$\endgroup\$ – firda Sep 18 '18 at 10:58
  • 1
    \$\begingroup\$ @firda, thanks! One thing is that i need shimming primarily for .net framework objects like MenuItem (objects where i cannot change the code inside). The other thing is that i heard conditional compilation to be some variety of a code smell. Therefore, the reason for shimming could be to avoid repeated usage of conditional compilation, but only to have it in special PortabilityLibrary. \$\endgroup\$ – Andrey K. Sep 18 '18 at 11:45
  • 1
    \$\begingroup\$ @t3chb0t There is code and my first two sections adressed exactly that (passing the real object to constructor and type checking in methods instead of in the constructor). The rest is of course about alternatives, because the question is kinda about design rather than actual code. But I have recently posted my own question and it turned out the code in fact did not work (I have overlooked integral promotion). There is a difference between the two, but my own question was in fact in the same category - not working or not implemented. The fate was different. I obviously do not see the line. \$\endgroup\$ – firda Sep 19 '18 at 7:28
  • 1
    \$\begingroup\$ @t3chb0t In this question though, the example is example usage. The rest of the code is real. Consider it example input even. \$\endgroup\$ – Mast Sep 19 '18 at 7:57
  • 1
    \$\begingroup\$ @t3chb0t Take the code upto no more for review and you can compile it (if you define NETFULL or use the .csproj part seen a bit later), so it is working code. But I am not 100% sure it belongs to CR, maybe it belongs to Software Engineering, maybe, because the core is more theoretical and about design, but it still is working code and there were things worth reviewing (about the code, not the design). I personally do not care about points, but a downvote is a signal and I am processing it, trying to find the line, become good reviewer or stop trying. That should be the purpose of downvote. \$\endgroup\$ – firda Sep 19 '18 at 11:37
3
\$\begingroup\$

Does it work / solve the problem?

public ExampleClassShim(object obj):base(obj) { }
...
var x = new ExampleClassShim(new NetCoreExampleClass());

Creating the Shim by passing the real object into constructor is defeating the purpose. But I can imagine you wanted something like this:

public ExampleClassShim(/*maybe args*/): base(
#if NETFULL
    new NetFullExampleClass(/*args*/)
#else
    new NetCoreExampleClass(/*args*/)
#endif
) {}

Why runtime type checks in methods?

        public ExampleClassShim(object obj):base(obj)
        {
#if NETFULL
            if (!(obj is ExampleClass))
                throw new InvalidImplementationException(); // custom exception
        ...

        public void ExampleMethod(string line)
        {
#if NETFULL
            ((ExampleClass)Obj).ExampleMethod(line);
        ...

How about generics?

public abstract class Shim<Impl>
{
    protected Impl It { get; }
    protected Shim(Impl it) { It = it; }
}

public class ExampleShim: Shim<
#if NETFULL
NetFullExample
...

...but that will introduce problems with inheritance, so...

Why not interfaces?

public interface IShimOne
{
    void MethodOne();
}
public interface IShimTwo: IShimOne
{
    void MethodTwo();
}
#if NETFULL
class One: RealOne, IShimOne {}
class Two: RealTwo, IShimTwo {}
public static class ShimFactory
{
    public static IShimOne CreateOne() { return new One(); }
    public static IShimTwo CreateTwo() { return new Two(); }
}

Need objects for inheritance?

public class WrapperOne
{
    protected IShimOne It { get; }
    protected WrapperOne(IShimOne it) { It = it; }
    public WrapperOne() { It = ShimFactory.CreateOne(); }
    public void MethodOne() { It.MethodOne(); }
}
public class WrapperTwo: WrapperOne
{
    protected new IShimTwo It => (IShimTwo)base.It;
    protected WrapperTwo(IShimTwo it): base(it) {}
    public WrapperTwo(): base(ShimFactory.CreateTwo()) {}
    public void MethodTwo() { It.MethodTwo(); }

Added after being accepted

The final version may look similar to the original, but the implementation (class One: RealOne, IShimOne + ShimFactory) can be separated to per-arch library, completely avoiding conditional compilation (if the factory is interfaced or called through reflection). This solution can be used not only for .net standard vs. .net core but also e.g. WinForms vs. WPF vs. GTK# and similar problems needing common API but different implementations based on architecture or available toolkits. I got the idea from Eto.Forms (where the handlers are also a wrapper and not derived and the per-arch libraries are embedded, but the core idea is the same).

It also solves the main problems with the code presented - creation and type safety (the .ctor accepting interface is protected as well as It that is safely casted to proper version. Safety is ensured by the protected .ctor but that scheme has to be followed in every derived wrapper, maybe even using WrapperTwo: this(ShimFactory.CreateTwo()) {} to be extra safe).

\$\endgroup\$
  • \$\begingroup\$ Thanks! Today is my birthday and it is a good present! I'll use generics for non-inheritance cases and interfaces for inheritance-cases. I think that writing visual studio snippets is a good idea. \$\endgroup\$ – Andrey K. Sep 18 '18 at 22:01
  • \$\begingroup\$ Do you know, Is it possible to use any ready wrappers for WPF and WinForms controls from Eto.Forms? It would be nice to take ready wrappers instead of creating custom ones.. \$\endgroup\$ – Andrey K. Sep 18 '18 at 22:03
  • \$\begingroup\$ @AndreyK.: As for the state of Eto.Forms - try it. It has been a while I was working with it (and actually on it, I have contributed splitters and some transparent background drawing logic to Eto), so, I am not sure about the state, but think it is production-grade ready for desktop (not so for mobile development). Maybe you can also consider Xamarin.Forms. \$\endgroup\$ – firda Sep 19 '18 at 6:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.