1
\$\begingroup\$

I have one interface for device but the two classes I have using the interface also include a list of a class. The classes the list are using are of ICommand. This gives me a problem because I run into multiple of essentially the same methods as seen below with the average methods. I'm fairly new to C# so I'm wondering if there is some way I can use an abstract class or something to eliminate all this duplicate code.

public interface IDevice
{
    double AvgCmdsMin { get; set; }
    double AvgTime { get; set; }
    string DeviceName { get; set; }
    double Duration { get; set; }
    int FailedCmds { get; set; }
    double MaxTime { get; set; }
    double MinTime { get; set; }
}

public class MuxDevice : IDevice
{
    public string DeviceName { get; set; } /*Name of Device*/
    public bool Active = false; /*True if device is currently waiting for a command response, false if not*/
    public List<MuxCmd> UsedCommands = new List<MuxCmd>(); /*List of all commands used by device*/
    public double MaxTime { get; set; } /*the longest time it took to receive a command response*/
    public double MinTime { get; set; } /*the shortest time it took to receive a command response*/
    public double AvgTime { get; set; } /*The average between all times to receive to a response*/
    public double Duration { get; set; } /*Total duration of all commands on device*/
    public double AvgCmdsMin { get; set; } /*How many commands were sent in a minute*/
    public int FailedCmds { get; set; } /*Number of commands that received an error*/
}

public class Device : IDevice
{
    public string DeviceName { get; set; } /*Name of Device*/
    public bool Active = false; /*True if device is currently waiting for a command response, false if not*/
    public List<Command> UsedCommands = new List<Command>(); /*List of all commands used by device*/
    public double MaxTime { get; set; } /*the longest time it took to receive a command response*/
    public double MinTime { get; set; } /*the shortest time it took to receive a command response*/
    public double AvgTime { get; set; } /*The average between all times to receive to a response*/
    public double Duration { get; set; } /*Total duration of all commands on device*/
    public double AvgCmdsMin { get; set; } /*How many commands were sent in a minute*/
    public int FailedCmds { get; set; } /*Number of commands that received an error*/
}

public interface ICommand
{
    string CmdType { get; set; }
    DateTime EndTime { get; set; }
    TimeSpan Length { get; set; }
    int RespBytes { get; set; }
    int RespCmd { get; set; }
    int SendBytes { get; set; }
    int SendCmd { get; set; }
    DateTime SendTime { get; set; }
}

private void AverageTime(List<Device> deviceStats)
{
    foreach (Device t in deviceStats)
    {
        long totalTicks = 0; /*total number of ticks(10,000 ticks in a millisecond)*/

        foreach (Command t1 in t.UsedCommands)
        {
            totalTicks += t1.Length.Ticks; /*adds up command lengths as ticks*/
        }

        TimeSpan totalDuration = new TimeSpan(totalTicks);
        t.Duration = totalDuration.TotalSeconds;

        if ((t.UsedCommands.Count - t.FailedCmds) == 0) /*if 0 commands*/
        {
            t.AvgTime = 0; /*Add avgTimeSpan to Device in seconds*/
        }
        else
        {
            var avgticks = totalTicks / (t.UsedCommands.Count - t.FailedCmds); /*Average ticks*/
            var avgTimeSpan = new TimeSpan(avgticks); /*Convert totalTicks to Timespan*/
            t.AvgTime = avgTimeSpan.TotalSeconds; /*Add avgTimeSpan to Device in seconds*/
        }
    }
}

private void MuxAverageTime(List<MuxDevice> deviceStats)
{
    foreach (MuxDevice t in deviceStats)
    {
        long totalTicks = 0; /*total number of ticks(10,000 ticks in a millisecond)*/

        foreach (MuxCmd t1 in t.UsedCommands)
        {
            totalTicks += t1.Length.Ticks; /*adds up command lengths as ticks*/
        }

        TimeSpan totalDuration = new TimeSpan(totalTicks);
        t.Duration = totalDuration.TotalSeconds;

        if ((t.UsedCommands.Count - t.FailedCmds) == 0) /*if 0 commands*/
        {
            t.AvgTime = 0; /*Add avgTimeSpan to Device in seconds*/
        }
        else
        {
            var avgticks =
                totalTicks / (t.UsedCommands.Count - t.FailedCmds); /*Average ticks*/
            var avgTimeSpan = new TimeSpan(avgticks); /*Convert totalTicks to Timespan*/
            t.AvgTime = avgTimeSpan.TotalSeconds; /*Add avgTimeSpan to Device in seconds*/
        }
    }
}
\$\endgroup\$
1
\$\begingroup\$

It's actually not uncommon that you have both the interface and a class that implements it the way you did it.

In order to make the interface reusable for both case you need to make use of generics and add a new generic parameter to IDevice that the list will be using. It would be a good idea to constrain it too with where on ICommand

public interface IDevice<TCommand> where TCommand : ICommand
{
    List<TCommand> UsedCommands { get; set; }
    // other members
}

Then you also need to modify the AverageTime to pass that generic parameter to the argument. The same constraint applies to this method now.

private void AverageTime<TCommand>(List<IDevice<TCommand>> devices) where TCommand : ICommand
{
    // ...
}

There is one more issue that you should address. The AvarageTime method modifies the parameter by altering the AvgTime.

t.AvgTime = 0;

If you do this you should rename the method to UpdateAverageTime or return a new list that contains objects with new values without modifying the input.

\$\endgroup\$
0
\$\begingroup\$

Looking at the code you've provided...

  • The two Device classes look exactly the same (except the UsedCommands), and therefore I don't see a need to have them as different classes at all.
  • These look like they're Models - and therefore don't need an Interface, like this at all. (I've certainly seen that before - and it's just weird) - Models don't need to be represented as Interfaces. (NB. That's not to say that there aren't good use cases for interfaces on models).
  • The UsedCommands List<Command> & List<MuxCmd> could be replaced with List<ICommand> (or just use List<Command> - another model), and use a single class.
  • The AverageTime methods, could just take the new List<ICommand> (or List<Command>), as unless I'm missing something, them do exactly the same thing.

Caveats: If there is more going on in these classes, that you're not showing, then it could make more sense.

\$\endgroup\$
0
\$\begingroup\$

Whether you should use inheritance (either of a concrete base class, or an abstract class) or not or even whether you need two classes, depends upon a lot of factors, and there's not enough details here to say.

But there is an alternative...extension methods on an Interface. They would allow you to have just one concrete implementation, that acted upon both.

\$\endgroup\$
0
\$\begingroup\$

Use an abstract class for all the common properties

public abstract class Device
{
    double AvgCmdsMin { get; set; }
    double AvgTime { get; set; }
    string DeviceName { get; set; }
    double Duration { get; set; }
    int FailedCmds { get; set; }
    double MaxTime { get; set; }
    double MinTime { get; set; }
}

public class MuxDevice : Device
{
    public List<MuxCmd> UsedCommands = new List<MuxCmd>(); /*List of all commands used by device*/
}

You could use an abstract class for the common commands and have just one AverageTime. For that matter you could use iCommand and have just one average time.

\$\endgroup\$

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.