5

The following code snippets are simplified to demonstrate the context! The actual interfaces and classes are POCOs having additional properties. The types are part of library I am working on, the interfaces are public API, the classes internal API.

I have:

  1. an interface IValue

    public interface IValue
    {
        Int32 Value { get; }
    }
    
  2. 2 different IConfigurableValue interfaces (2 separate git branches) which extend IValue, see further on

    public interface IConfigurableValue : IValue
    {
        void CopyFrom(IValue source); // is always part of the interface
    }
    
  3. an implementation AutoGeneratedValue

    internal class AutoGeneratedValue : IConfigurableValue
    {
        public Int32 Value { get; set; }
    
        public void CopyFrom(IValue source)
        {
            Value = source.Value;
        }
    }
    
  4. code which needs to access the getter only (via IValue) and code which requires a IConfigurableValue (no compile-time known implementation)

    External users of the library can implement IValue and IConfigurableValue on their own. Part of my library code creates an instance of AutoGeneratedValue. If the user provides a custom IConfigurableValue implementation my AutoGeneratedValue is passed to it, the user decides if to ignore it or not. Another API part accepts an instance of IValue and does something. It only needs to read the instance properties. This part of the API can be invoked without ever requiring any IConfigurableValue instances.

My goal for the library is a) to offer methods requiring an IValue instance and b) methods to auto-generated IValue instances or populate user-provided IConfigurableValue instances which can be used for part a.

I currently have 2 branches with the following IConfigurableValue definitions:

  • IConfigurableValue will extend a separate IValueConfiguration as well as IValue

    public interface IConfigurableValue : IValue, IValueConfiguration
    {
        void CopyFrom(IValue source);
    }
    
    public interface IValueConfiguration
    {
        Int32 Value { set; }
    }
    
  • and a IConfigurableValue which defines the setter itself, no additional interface

    public interface IConfigurableValue : IValue
    {
        new Int32 Value { set; }
    
        void CopyFrom(IValue source);
    }
    

With both options I need to cast IConfigurableValue to IValue to access the getter in part b of my library. With option no. 1 I need to cast IConfigurableValue to IValueConfiguration to access the setter.

Option no. 2 denies me the usage via a setter-only interface because any IConfigurableValue is an IValue, with no. 1 I could use the IValueConfiguration if I need it. -- This is no actual requirement of my API but I don't know the future demands for the API.

Is there another option to achieve my goal and which option should be preferred?

gnat
  • 21,442
  • 29
  • 112
  • 288
ckerth
  • 51
  • 4
  • 2
    @ckerth Yes, but I have to give you props for making this a good question though. I hope it will be well received on programmers. – Bruno Costa Aug 25 '16 at 13:21
  • @SimonForsberg That's why I consider this a *best-practice* question. I have 2 solutions, both working but having some drawbacks. Maybe there is a better solution I don't know of yet. I don't want opinion like "I like that better", I want to know if one or the other option should be used (or preferred) for a reason. or to shorten "best practice" – ckerth Aug 26 '16 at 09:25
  • Refresh my memory. What happens if you make `IConfigurableValue : IValue { Int32 Value { get; set; } }` ? Does that compile? – RubberDuck Aug 26 '16 at 10:31
  • @RubberDuck first off: [CS0108](https://msdn.microsoft.com/en-us/library/3s8070fc.aspx) -- but yes it should compile. I don't want it it for a reason of explicit implementation for `get` via both interfaces. Example: `public class Sample : IConfigurableValue { Int32 IConfigurableValue.Value { get; set; } Int32 IValue.Value { get; } }` – ckerth Aug 26 '16 at 11:03
  • @ckerth it might not be pretty *inside* your implementation, but it's probably the cleanest solution for the client code. I'd argue it matters way more how it gets called than implemented. An "ugly" implementation gets contained. An ugly interface (API not the keyword `interface`) pollutes your code base. – RubberDuck Aug 26 '16 at 11:46
  • @RubberDuck I see your point. On the other hand an API is a contract "how to use my library". If a client implementation is clean doesn't matter to me, he has to fulfill my contract (that's what matters imho). As a side note, implementing `IConfigurableValue` is simple: auto-property POCO, applies to your proposed interface same as my 2 solutions. I will consider your proposed solution nonetheless, thanks. – ckerth Aug 26 '16 at 11:56
  • You're confusing children and clients @ckerth. – RubberDuck Aug 26 '16 at 13:35
  • 1
    Is there a reason that `IConfigurableValue` needs to extend `IValue`? You could just have them as two different interfaces implemented by the same class – Ben Aaronson Aug 26 '16 at 15:25
  • @BenAaronson Uhmm, that's the gist of the question. I **have** `IValue` and I want to extend it and how is this achieved properly. Because this question was formerly posted on *codereview* I have included actual code snippets. – ckerth Aug 29 '16 at 09:12
  • @ckerth Right, I'm just wondering if maybe your needs would be served as well/better by having two separate interfaces where one does not extend the other. (i.e. `IValue` has `Value {get;}`, `IConfgurableValue` has `Value {get; set;}`, and you write `AutoGeneratedValue : IValue, IConfigurableValue`) – Ben Aaronson Aug 29 '16 at 13:43
  • @BenAaronson With this approach part B of my library doesn't require the `IValue` to populate an existing `IConfigurableValue`. And the auto-generated field could be returned as `IValue`. So yes, for my API it can be done that way as an alternative. Side note: like RubberDuck' suggestion, different explicit implementations of `get` are possible. You may want to consider writing an answer for other users stumbling over this question. (because the gist of your answer is "don't extend it") – ckerth Aug 29 '16 at 14:28

1 Answers1

1

Consider just doing this:

public interface IValue
{
    Int32 Value { get; }
}

public interface IConfigurableValue : IValue
{
    void SetValue(Int32 newValue);
}

The obvious disadvantage here is that callers of IConfigurableValue have to use an unusual, Java-like syntax for the setter instead of the normal C# syntax. Nevertheless, this solution is simple and easy to understand.

You should only define CopyFrom as an interface member if its implementation may vary between different implementing classes. If all implementing classes will use the same implementation of CopyFrom, just define it as an extension method instead:

public static class ConfigurableValueExtensions
{
    public static void CopyFrom(this IConfigurableValue destination, IValue source)
    {
        destination.SetValue(source.Value);
    }
}
Tanner Swett
  • 1,359
  • 8
  • 15