0

Is the code below fine? I'm asking myself if the Start() and Stop() method in the interface ICustomTimer is fine as location. Because I need it in my Main method.

Is this a code smell or, in other words, what is the best practice to call a base method which has no abstraction? Timer class has no interface that I can use to inherit from.

public interface ICustomTimer
{
    string Value { get; set; }

    //Implementation in Timer
    void Start();

    //Implementation in Timer
    void Stop();
}

public class CustomTimer : System.Timers.Timer, ICustomTimer
{
    public string Value { get; set; }
}

public Main()
{
    var customTimerObj = iocContainer.Get<ICustomTimer>();
    customTimerObj.Start();
}
Ozkan
  • 3,880
  • 9
  • 47
  • 78
  • 3
    The only problem I see is that you don´t have any turther possibility of inheritance, that´s why we [favour composition over inheritance](https://stackoverflow.com/questions/49002/prefer-composition-over-inheritance). Apart from this it´s just fine, but that´s quite opinion-based and will therefore probably be closed. – MakePeaceGreatAgain Jun 20 '18 at 12:44
  • 2
    This does leave you a little tied in to the implementation of `System.Timers.Timer`. Let's say someone at MS has a brain fart and changes the method to `StartTimer`, then your code will break and you'll be forced to implement the `Start` method anyway. Also, you have the possibility that someone would try to access the `Timer` methods directly. I would prefer to wrap the timer inside so it's completely encapsulated. But as @HimBromBeere says, this is all opinion... – DavidG Jun 20 '18 at 12:47
  • 1
    'fine' is a little subjective, but it is valid. Do note that when you use another Timer class you will have to map their equivalents of Start & Stop. – bommelding Jun 20 '18 at 12:49
  • 1
    @DavidG - that brainfart has already happened, multiple time(r)s. – bommelding Jun 20 '18 at 12:50

1 Answers1

1

It's a valid use, a good use even, if all you whant is to call the class method when you use the interface, otherwise you could do this:

public interface ICustomTimer
{
    string Value { get; set; }

    //Implementation in Timer
    void Start();

    //Implementation in Timer
    void Stop();
}

public class CustomTimer : System.Timers.Timer, ICustomTimer
{
    public string Value { get; set; }
    void ICustomTimer.Start() { this.Start(); }
    void ICustomTimer.Stop() { this.Stop(); }
}

That way you can do something else (prior or post the method call to the Timer class)

SammuelMiranda
  • 420
  • 4
  • 29
  • 1
    Why is it a "good use" then? The question is opinion based and so is this answer I'm afraid. – DavidG Jun 20 '18 at 12:59
  • 1
    is good because is simple, fits the purpuse and it's in complience with the use of Interfaces as described in MSDN documentation, i also presented an alternative, to make clear what it would do and useful only if you'd want to add steps prior and post the method call (like raise an event first, for example) – SammuelMiranda Jun 20 '18 at 13:08
  • 1
    also, to cover your concern DavigG on your other comment "Let's say someone at MS has a brain fart and changes the method to StartTimer", it wouldn't happen because his project would be bound to a version of .net framework at compile time - any changes by Microsoft would be in new releases of .net, not changing the version that he would bind on during developement. – SammuelMiranda Jun 20 '18 at 13:11