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

Next Instance of DRY Refactoring

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

Problem

I'd like some additional assistance on refactoring. This is my latest update for this iteration.

```
#region WMI Classes
///
/// The Win32_OperatingSystem WMI class represents a Windows-based OS installed on a computer. Any OS that can be
/// installed on a computer that can run a Windows-based OS is a descendent or member of this class.
/// Win32_OperatingSystem is a singleton class. To get the single instance, use "@" for the key.
///
public class Win32OperatingSystem
{

public Win32OperatingSystem() { }

PropertyGetter pg = new PropertyGetter("Win32_OperatingSystem");

///
/// Number, in megabytes, of physical memory currently unused and available.
///
public ulong FreePhysicalMemory()
{
return pg.GetUnsignedInt64("FreePhysicalMemory");
}

///
/// Number, in megabytes, of virtual memory currently unused and available.
///
public ulong FreeVirtualMemory()
{
return pg.GetUnsignedInt64("FreeVirtualMemory");
}

///
/// Number, in megabytes, of virtual memory.
///
public ulong TotalVirtualMemorySize()
{
return pg.GetUnsignedInt64("TotalVirtualMemorySize");
}
}

///
/// The Win32_ComputerSystem WMI class represents a computer system running Windows.
///
public class Win32ComputerSystem
{

public Win32ComputerSystem() { }

PropertyGetter pg = new PropertyGetter("Win32_ComputerSystem");

///
/// Key of a CIM_System instance in an enterprise environment.
/// This property is inherited from CIM_ManagedSystemElement.
///
public string Name()
{
return pg.GetString("Name");
}

///
/// Name of a computer manufacturer.
///
public string Manufacturer()
{
return pg.GetString("Manufacturer");
}

///
/// Product name that a manufacturer gives to a computer. This property must have a value.
///
public string Model()
{
return pg.GetString("Model");
}
}

//

Solution

-
What is the point of empty public constructors? If there's no logic in them, remove them.

-
The PropertyGetter class member in multiple classes is never assigned to after it is initialized. Mark them readonly. Same goes for _win32Class in the PropertyGetter class itself.

-
There are a number of methods which follow a particular pattern in the PropertyGetter class. I am a fan of brevity and fewer lines of code, so I made it look like this (for example):

public DateTime GetDateTimeFromDmtf(string propertyName)
{
    using (var searcher = new ManagementObjectSearcher("SELECT " + propertyName + " FROM " + this._win32Class))
    using (var collection = searcher.Get())
    using (var enu = collection.GetEnumerator())
    {
        return !enu.MoveNext() || (enu.Current[propertyName] == null)
            ? DateTime.Today
            : ManagementDateTimeConverter.ToDateTime(enu.Current[propertyName].ToString());
    }
}


-
Since all these things have the word "property" in them, possibly make them properties instead of methods? Example:

/// 
/// Manufacturer of this software element.
/// 
public string Manufacturer()
{
    return this.pg.GetString("Manufacturer");
}


becomes:

/// 
    /// Manufacturer of this software element.
    /// 
    public string Manufacturer
    {
        get
        {
            return this.pg.GetString("Manufacturer");
        }
    }

Code Snippets

public DateTime GetDateTimeFromDmtf(string propertyName)
{
    using (var searcher = new ManagementObjectSearcher("SELECT " + propertyName + " FROM " + this._win32Class))
    using (var collection = searcher.Get())
    using (var enu = collection.GetEnumerator())
    {
        return !enu.MoveNext() || (enu.Current[propertyName] == null)
            ? DateTime.Today
            : ManagementDateTimeConverter.ToDateTime(enu.Current[propertyName].ToString());
    }
}
/// <summary>
/// Manufacturer of this software element.
/// </summary>
public string Manufacturer()
{
    return this.pg.GetString("Manufacturer");
}
/// <summary>
    /// Manufacturer of this software element.
    /// </summary>
    public string Manufacturer
    {
        get
        {
            return this.pg.GetString("Manufacturer");
        }
    }

Context

StackExchange Code Review Q#43719, answer score: 6

Revisions (0)

No revisions yet.