HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Basic network utilisation display

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
utilisationdisplaynetworkbasic

Problem

I've got the following code to monitor the network usage on a network interface. I'm mainly looking for feedback on the design, but if you see any code improvements please let me know! This is also the first time I've actually added 'official' documentation so tell me if I'm forgetting something.
For those that feel more comfortable browsing github: you can find the code here.

Statistics:

public interface IStatistics {
    string NetworkInterface { get; }
    float DataSent { get; }
    float DataReceived { get; }
    float UploadSpeed { get; }
    float DownloadSpeed { get; }
    Queue LatestDownTransfers { get; }
    Queue LatestUpTransfers { get; }
}

public class Statistics : IStatistics {
    public Statistics(string name) {
        NetworkInterface = name;
        LatestDownTransfers = new Queue(3);
        LatestUpTransfers = new Queue(3);
    }

    // 
    // Holds the name of the selected network interface
    // 
    public string NetworkInterface { get; set; }

    // 
    // Contains the data sent in the most recent time interval
    // 
    public float DataSent { get; set; }

    // 
    // Contains the data received in the most recent time interval
    // 
    public float DataReceived { get; set; }

    // 
    // Returns the upload speed in KiloBytes / Second
    // 
    public float UploadSpeed {
        get { return LatestUpTransfers.Sum() / LatestUpTransfers.Count / 1028 / StatisticsFactory.MULTIPLIER; }
    }

    // 
    // Returns the download speed in KiloBytes / Second
    // 
    public float DownloadSpeed {
        get { return LatestDownTransfers.Sum() / LatestDownTransfers.Count / 1028 / StatisticsFactory.MULTIPLIER; }
    }

    // 
    // Contains the data received in the three most recent time intervals
    // 
    public Queue LatestDownTransfers { get; set; }

    // 
    // Contains the data sent in the three most recent time intervals
    // 
    public Queue LatestUpTransfers { get; set; }
}


Statistics Factory:

```
p

Solution

Style

  • IStatistics is a bit of a generic name (although I guess if you put it in a sensible namespace it might be ok). I'd consider renaming it to something like INetworkStatistics.



  • From looking at the code it is not obvious what units the upload and download speeds are (apart from one comment). Consider abstracting it behind a TransferSpeed class from which you can get any sensible transfer speed (e.g. KB/sec, MB/sec, MB/min, ...) or at least put the unit in the property name.



Design

-
Your interface IStatistics exposes some internals which should not be of any interest to the user, namely DataSent, DataReceived and LatestDownTransfers, LatestUpTransfers. I'd say the user only cares about the fact that he can get the up/download rate and add new data points. So your interface should look like this:

public interface INetworkStatistics 
{
    string NetworkInterface { get; }
    float UploadSpeed { get; }
    float DownloadSpeed { get; }
    void AddSentData(float data);
    void AddReceivedData(float data);
}


You can then implement the Add methods as you implemented them in your factory class.

-
Rename your utility function GetNetworkStatistics into UpdateNetworkStatistics and pass it as parameter an IStatistics instance to record the data points for:

public static void UpdateNetworkStatistics(IStatistics stats)
{
    var dataSentCounter = new PerformanceCounter("Network Interface", "Bytes Sent/sec", stats.NetworkInterface);
    var dataReceivedCounter = new PerformanceCounter("Network Interface", "Bytes Received/sec", stats.NetworkInterface);

    float sendSum = 0;
    float receiveSum = 0;

    for (var index = 0; index  0 || receiveSum > 0)
    {
        stats.AddReceivedData(receiveSum);
        stats.AddSentData(sendSum);
    }
}


The while(true) loop in your main program would now become:

stats = new Statistics(instances[choice]);
while (true) 
{
    Utilities.UpdateNetworkStatistics(stats);
    ...


  • Lo and behold you can now delete the StatisticsFactory class. It serves no purpose other than making it near impossible to get statistics for more than one interface at a time and interfere with unit testing big time.

Code Snippets

public interface INetworkStatistics 
{
    string NetworkInterface { get; }
    float UploadSpeed { get; }
    float DownloadSpeed { get; }
    void AddSentData(float data);
    void AddReceivedData(float data);
}
public static void UpdateNetworkStatistics(IStatistics stats)
{
    var dataSentCounter = new PerformanceCounter("Network Interface", "Bytes Sent/sec", stats.NetworkInterface);
    var dataReceivedCounter = new PerformanceCounter("Network Interface", "Bytes Received/sec", stats.NetworkInterface);

    float sendSum = 0;
    float receiveSum = 0;

    for (var index = 0; index < Statistics.MULTIPLIER; index++)
    {
        sendSum += dataSentCounter.NextValue();
        receiveSum += dataReceivedCounter.NextValue();
    }

    if (sendSum > 0 || receiveSum > 0)
    {
        stats.AddReceivedData(receiveSum);
        stats.AddSentData(sendSum);
    }
}
stats = new Statistics(instances[choice]);
while (true) 
{
    Utilities.UpdateNetworkStatistics(stats);
    ...

Context

StackExchange Code Review Q#28367, answer score: 5

Revisions (0)

No revisions yet.