5
\$\begingroup\$

This should be the last iteration for this code.

  1. Does this approach make sense or am I heading down the wrong path?

  2. The only thing I can see to make this better is to genericize the GetType methods at the end so that I could eliminate the goofy if statement in the GetProperties method. Any suggestions?

What I've done between the last question and now:

I've used a private method GetProperty as suggested in the last question in order to minimize the repeated code.

private object GetProperty(string propertyName)
{
    using (ManagementObjectSearcher moSearcher = new ManagementObjectSearcher
        ("SELECT " + propertyName + " FROM " + WmiClassName))
    using (var collection = moSearcher.Get())
    using (var enu = collection.GetEnumerator())
        return (!enu.MoveNext() || enu.Current[propertyName] == null) ? null : enu.Current[propertyName];
}

Step 1

I decided to edit the PropertyValue class along these lines to handle the dictionary

public PropertyValue(Dictionary<string, string> propertyList, string wmiClassName).

Where the first string in the dictionary is the type the property will return and second in the dictionary is the propertyName as a string so that it is easy to work with. I'd like to genericize the type part so I could use a list or array instead of a dictionary. Really just to eliminate one of the string variables.

Step 2

I updated the calls to the WMIClasses:

public class Win32Processor
{

    public Win32Processor() { }

    readonly PropertyValue propertyValue = new PropertyValue("Win32_Processor");

    /// <summary>
    /// Processor architecture used by the platform.
    /// </summary>
    public ushort Architecture()
    { return propertyValue.GetUnignedInt16("Architecture"); }

    /// <summary>
    /// Description of the object. This property is inherited from CIM_ManagedSystemElement.
    /// </summary>
    public string Description()
    { return propertyValue.GetString("Description"); }
}

To...

public class Win32BaseBoard
{

public object GetValues()
    {
    Dictionary<string, string> propertyList = new Dictionary<string, string>(){

        /// <summary>
        /// 
        /// </summary>
        {"ushort", "AddressWidth"},

        /// <summary>
        /// 
        /// </summary>
        {"ushort", "Architecture"},};

        // ...

        PropertyValue propertyValue = new PropertyValue(propertyList, "Win32_BaseBoard");
        return propertyValue.GetProperties();
    }
}

Then I went about refactoring my PropertyValue class.

After implementing all of the above here's the full code as it stands:

#region WMI Classes

/// <summary>
/// The Win32_BaseBoard WMI class represents a baseboard, which is also known as a motherboard or system board.
/// </summary>
public class Win32BaseBoard
{
    public Win32BaseBoard() { }

    public object GetValues()
    {
        Dictionary<string, string> propertyList = new Dictionary<string, string>(){

            {"ushort", "AddressWidth"},

            {"ushort", "Architecture"},

            {"ushort", "Availability"},

            {"ushort", "CpuStatus"},

            {"uint", "CurrentClockSpeed"},

            {"ushort", "DataWidth"},

            {"string", "Description"},


            {"string", "DeviceID"},

            {"uint", "ExtClock"},


            {"ushort", "Family"},


            {"uint", "L2CacheSize"},


            {"uint", "L2CacheSpeed"},


            {"ushort", "Level"},

            {"ushort", "LoadPercentage"},


            {"string", "Manufacturer"},


            {"uint", "MaxClockSpeed"},


            {"string", "Name"},


            {"uint", "NumberOfCores"},

            {"string", "PNPDeviceID"},

            {"string", "ProcessorId"},

            {"ushort", "ProcessorType"},

            {"ushort", "Revision"},

            {"string", "Role"},

            {"string", "SocketDesignation"},

            {"ushort", "StatusInfo"},

            {"string", "Stepping"},

            {"string", "UniqueId"},

            {"ushort", "UpgradeMethod"},

            {"string", "Version"},

            {"uint", "VoltageCaps"},
        };
        PropertyValue propertyValue = new PropertyValue(propertyList, "Win32_BaseBoard");
        return propertyValue.GetProperties();
    }
}

/// <summary>
/// The Win32_BIOS WMI class represents the attributes of the computer system's basic input/output services (BIOS) 
/// that are installed on a computer.
/// </summary>
public class Win32BIOS
{

    public Win32BIOS() { }

    public object GetValues()
    {
        Dictionary<string, string> propertyList = new Dictionary<string, string>(){

            {"string", "BankLabel"},

            {"string", "BuildNumber"},

            {"ulong", "Capacity"},

            {"string", "CurrentLanguage"},

            {"ushort", "DataWidth"},

            {"string", "Description"},

            {"ushort", "FormFactor"},

            {"bool", "HotSwappable"},

            {"string", "InstallableLanguages"},

            {"string", "Manufacturer"},

            {"ushort", "MemoryType"},

            {"string", "Name"},

            {"string", "PartNumber"},

            {"uint", "PositionInRow"},

            {"datetime", "ReleaseDate"},

            {"string", "SerialNumber"},

            {"string", "SMBIOSMajorVersion"},

            {"string", "SMBIOSMinorVersion"},

            {"bool", "SMBIOSPresent"},

            {"string", "SMBIOSBIOSVersion"},

            {"uint", "Speed"},

            {"string", "Status"},

            {"string", "Tag"},

            {"ushort", "TypeDetail"},

            {"string", "Version"}
        };
        PropertyValue propertyValue = new PropertyValue(propertyList, "Win32_BIOS");
        return propertyValue.GetProperties();
    }


}

/// <summary>
/// The Win32_DiskDrive WMI class represents a physical disk drive as seen by a computer running the Windows 
/// operating system. Any interface to a Windows physical disk drive is a descendent (or member) of this class. 
/// The features of the disk drive seen through this object correspond to the logical and management 
/// characteristics of the drive. In some cases, this may not reflect the actual physical characteristics 
/// of the device. Any object based on another logical device would not be a member of this class.
/// For security reasons, a user connecting from a remote computer must have the SC_MANAGER_CONNECT privilege 
/// enabled to be able to enumerate this class.
/// </summary>
public class Win32DiskDrive
{
    public Win32DiskDrive() { }

    public object GetValues()
    {
        Dictionary<string, string> propertyList = new Dictionary<string, string>(){

            /// <summary>
            /// 
            /// </summary>
            {"string", "Description"},

            /// <summary>
            /// 
            /// </summary>
            {"string", "Manufacturer"},

            /// <summary>
            /// 
            /// </summary>
            {"string", "Model"},

            /// <summary>
            /// 
            /// </summary>
            {"string", "Name"},

            /// <summary>
            /// 
            /// </summary>
            {"string", "OtherIdentifyingInfo"},

            /// <summary>
            /// 
            /// </summary>
            {"string", "PartNumber"},

            /// <summary>
            /// 
            /// </summary>
            {"string", "Product"},

            /// <summary>
            /// 
            /// </summary>
            {"string", "RequirementsDescription"},

            /// <summary>
            /// 
            /// </summary>
            {"string", "SerialNumber"},

            /// <summary>
            /// 
            /// </summary>
            {"string", "SKU"},

            /// <summary>
            /// 
            /// </summary>
            {"string", "SlotLayout"},

            /// <summary>
            /// 
            /// </summary>
            {"bool", "SpecialRequirements"},

            /// <summary>
            /// 
            /// </summary>
            {"string", "Tag"},

            /// <summary>
            /// 
            /// </summary>
            {"string", "Version"}
        };
        PropertyValue propertyValue = new PropertyValue(propertyList, "Win32_DiskDrive");
        return propertyValue.GetProperties();
    }
}

/// <summary>
/// The Win32_DiskPartition WMI class represents the capabilities and management capacity of a partitioned area 
/// of a physical disk on a computer system running Windows. Example: Disk #0, Partition #1.
/// </summary>
public class Win32DiskPartion
{
    public Win32DiskPartion() { }

    public object GetValues()
    {
        Dictionary<string, string> propertyList = new Dictionary<string, string>(){

            /// <summary>
            /// 
            /// </summary>
            {"string", "Name"},

            /// <summary>
            /// 
            /// </summary>
            {"bool", "PrimaryBIOS"}
        };
        PropertyValue propertyValue = new PropertyValue(propertyList, "Win32_DiskPartion");
        return propertyValue.GetProperties();
    }
}

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

    public Win32ComputerSystem() { }

    public object GetValues()
    {
        Dictionary<string, string> propertyList = new Dictionary<string, string>(){
            /// <summary>
            /// Key of a CIM_System instance in an enterprise environment.
            /// This property is inherited from CIM_ManagedSystemElement.
            /// </summary>   
            {"string", "Name"},

            /// <summary>
            /// Name of a computer manufacturer.
            /// </summary>  
            {"string", "Manufacturer"},

            /// <summary>
            /// Product name that a manufacturer gives to a computer. This property must have a value.
            /// </summary>
            {"string", "Model"}
    };
        PropertyValue propertyValue = new PropertyValue(propertyList, "Win32_ComputerSystem");
        return propertyValue.GetProperties();
    }
}

/// <summary>
/// The Win32_LogicalDisk WMI class represents a data source that resolves to an actual local storage device 
/// on a computer system running Windows.
/// </summary>
public class Win32LogicalDisk
{
    public Win32LogicalDisk() { }

    public object GetValues()
    {
        Dictionary<string, string> propertyList = new Dictionary<string, string>(){

            /// <summary>
            /// 
            /// </summary>
            {"uint", "BytesPerSector"},

            /// <summary>
            /// 
            /// </summary>
            {"ushort", "Capabilities"},

            /// <summary>
            /// 
            /// </summary>
            {"string", "Caption"},

            /// <summary>
            /// 
            /// </summary>
            {"string", "DeviceID"},

            /// <summary>
            /// 
            /// </summary>
            {"suint", "Index"},

            /// <summary>
            /// 
            /// </summary>
            {"string", "InterfaceType"},

            /// <summary>
            /// 
            /// </summary>
            {"string", "Manufacturer"},

            /// <summary>
            /// 
            /// </summary>
            {"bool", "MediaLoaded"},

            /// <summary>
            /// 
            /// </summary>
            {"string", "MediaType"},

            /// <summary>
            /// 
            /// </summary>
            {"string", "Model"},

            /// <summary>
            /// 
            /// </summary>
            {"string", "Name"},

            /// <summary>
            /// 
            /// </summary>
            {"uint", "Partitions"},

            /// <summary>
            /// 
            /// </summary>
            {"string", "PNPDeviceID"},

            /// <summary>
            /// 
            /// </summary>
            {"uint", "SCSIBus"},

            /// <summary>
            /// 
            /// </summary>
            {"ushort", "SCSILogicalUnit"},

            /// <summary>
            /// 
            /// </summary>
            {"ushort", "SCSIPort"},

            /// <summary>
            /// 
            /// </summary>
            {"ushort", "SCSITargetId"},

            /// <summary>
            /// 
            /// </summary>
            {"uint", "SectorsPerTrack"},

            /// <summary>
            /// 
            /// </summary>
            {"uint", "Signature"},

            /// <summary>
            /// 
            /// </summary>
            {"ulong", "Size"},

            /// <summary>
            /// 
            /// </summary>
            {"string", "Status"},

            /// <summary>
            /// 
            /// </summary>
            {"ulong", "TotalCylinders"},

            /// <summary>
            /// 
            /// </summary>
            {"uint", "TotalHeads"},

            /// <summary>
            /// 
            /// </summary>
            {"ulong", "TotalSectors"},

            /// <summary>
            /// 
            /// </summary>
            {"ulong", "TotalTracks"},

            /// <summary>
            /// 
            /// </summary>
            {"uint", "TracksPerCylinder"}
        };
        PropertyValue propertyValue = new PropertyValue(propertyList, "Win32_LogicalDisk");
        return propertyValue.GetProperties();
    }
}

/// <summary>
/// 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.
/// </summary>
public class Win32OperatingSystem
{
    public object GetValues()
    {
        Dictionary<string, string> propertyList = new Dictionary<string, string>(){
            /// <summary>
            /// Number, in megabytes, of physical memory currently unused and available.
            /// </summary>    
            {"ulong", "FreePhysicalMemory"},
            /// <summary>
            /// Number, in megabytes, of virtual memory currently unused and available.
            /// </summary>    
            {"ulong", "FreeVirtualMemory"},
            /// <summary>
            /// Number, in megabytes, of virtual memory.
            /// </summary>
            {"ulong", "TotalVirtualMemory"}
        };
        PropertyValue propertyValue = new PropertyValue(propertyList, "Win32_OperatingSystem");
        return propertyValue.GetProperties();
    }
}

/// <summary>
/// The Win32_PhysicalMemory WMI class represents a physical memory device located on a computer system and 
/// available to the operating system.
/// </summary>
public class Win32PhysicalMemory
{
    public Win32PhysicalMemory() { }

    public object GetValues()
    {
        Dictionary<string, string> propertyList = new Dictionary<string, string>(){

            /// <summary>
            /// 
            /// </summary> 
            {"ulong", "BlockSize"},

            /// <summary>
            /// 
            /// </summary>
            {"bool", "Bootable"},

            /// <summary>
            /// 
            /// </summary>
            {"bool", "BootPartition"},

            /// <summary>
            /// 
            /// </summary>
            {"string", "Description"},

            /// <summary>
            /// 
            /// </summary>
            {"string", "DeviceID"},

            /// <summary>
            /// 
            /// </summary>
            {"ushort", "DiskIndex"},

            /// <summary>
            /// 
            /// </summary>
            {"ushort", "Index"},

            /// <summary>
            /// 
            /// </summary>
            {"string", "Name"},

            /// <summary>
            /// 
            /// </summary>
            {"ulong", "NumberOfBlocks"},

            /// <summary>
            /// 
            /// </summary>
            {"bool", "PrimaryPartition"},

            /// <summary>
            /// 
            /// </summary>
            {"ulong", "Size"},

            /// <summary>
            /// 
            /// </summary>
            {"ulong", "StartingOffset"},

            /// <summary>
            /// 
            /// </summary>
            {"string", "Type"}
        };

        PropertyValue propertyValue = new PropertyValue(propertyList, "Win32_PhysicalMemory");
        return propertyValue.GetProperties();
    }
}

/// <summary>
/// The Win32_Processor WMI class represents a device that can interpret a sequence of instructions on a computer 
/// running on a Windows operating system. On a multiprocessor computer, one instance of the Win32_Processor class 
/// exists for each processor.
/// </summary>
public class Win32Processor
{

    public Win32Processor() { }

    public object GetValues()
    {
        Dictionary<string, string> propertyList = new Dictionary<string, string>(){
            /// <summary>
            /// Key of a CIM_System instance in an enterprise environment.
            /// This property is inherited from CIM_ManagedSystemElement.
            /// </summary>   
            {"ushort", "Architecture"},

            /// <summary>
            /// Name of a computer manufacturer.
            /// </summary>  
            {"string", "Description"},

            /// <summary>
            /// 
            /// </summary>
            {"string", "DeviceID"},

            /// <summary>
            /// 
            /// </summary>
            {"uint", "DriveType"},

            /// <summary>
            /// 
            /// </summary>
            {"string", "FileSystem"},

            /// <summary>
            /// 
            /// </summary>
            {"uint", "MediaType"},

            /// <summary>
            /// 
            /// </summary>
            {"string", "Name"},

            /// <summary>
            /// 
            /// </summary>
            {"bool", "VolumeDirty"}
        };
        PropertyValue propertyValue = new PropertyValue(propertyList, "Win32_Processor");
        return propertyValue.GetProperties();
    }
}

#endregion

#region PropertyValue
/// <summary>
/// Handles the actual WMI queries for the other classes in this file
/// </summary>
public class PropertyValue
{

    public PropertyValue() { }

    public PropertyValue(Dictionary<string, string> propertyList, string wmiClassName)
    {
        this.PropertyList = propertyList;
        this.WmiClassName = wmiClassName;
    }

    public string WmiClassName { get; set; }
    public Dictionary<string, string> PropertyList { get; set; }

    /// <summary>
    /// Converts kilobyte values to megabytes for readability.
    /// </summary>
    /// <param name="kiloBytes">Value to be converted</param>
    /// <returns>Kilobytes converted to megabytes as ulong</returns>
    private static ulong KiloBytesToMegaBytes(ulong kiloBytes)
    { return kiloBytes / (ulong)1024; }

    public object GetProperties()
    {
        foreach (var pair in PropertyList)
        {
            if (pair.Key.ToLower() == "datetime")
            { return GetDateTimeFromDmtf(pair.Value); }
            if (pair.Key.ToLower() == "string")
            { return GetString(pair.Value); }
            if (pair.Key.ToLower() == "ulong")
            { return GetUnsignedInt64(pair.Value); }
            if (pair.Key.ToLower() == "bool")
            { return GetBool(pair.Value); }
            if (pair.Key.ToLower() == "ushort")
            { return GetUnignedInt16(pair.Value); }
            if (pair.Key.ToLower() == "uint")
            { return GetUnsignedInt32(pair.Value); }
        }
        return null;
    }

    private uint GetUnsignedInt32(string propertyName)
    {
        var property = FindProperty(propertyName);
        return property == null ? (uint)(0) : (uint)(property);
    }

    private object FindProperty(string propertyName)
    {
        using (ManagementObjectSearcher moSearcher = new ManagementObjectSearcher
            ("SELECT " + propertyName + " FROM " + WmiClassName))
        using (var collection = moSearcher.Get())
        using (var enu = collection.GetEnumerator())
            return (!enu.MoveNext() || enu.Current[propertyName] == null) ? null : enu.Current[propertyName];

    }

    private DateTime GetDateTimeFromDmtf(string propertyName)
    {
        var property = FindProperty(propertyName);
        return property == null ? DateTime.Today : ManagementDateTimeConverter.ToDateTime(property.ToString());
    }

    private string GetString(string propertyName)
    {
        var property = FindProperty(propertyName);
        return property == null ? String.Empty : property.ToString();
    }

    private ulong GetUnsignedInt64(string propertyName)
    {
        var property = FindProperty(propertyName);
        return property == null ? (ulong)(0) : KiloBytesToMegaBytes((ulong)(property));
    }

    private bool GetBool(string propertyName)
    {
        var property = FindProperty(propertyName);
        return property == null ? false : true;
    }

    private ushort GetUnignedInt16(string propertyName)
    {
        var property = FindProperty(propertyName);
        return property == null ? (ushort)(0) : (ushort)(property);
    }
}
#endregion

Questions in this series

Class Seperation vs Polymorphism.

First iteration of DRY refactoring.

Second iteration of DRY refactoring.

You are currently viewing the fourth question in the series.

\$\endgroup\$

2 Answers 2

6
\$\begingroup\$

First:

        if (pair.Key.ToLower() == "datetime")
        { return GetDateTimeFromDmtf(pair.Value); }
        if (pair.Key.ToLower() == "string")
        { return GetString(pair.Value); }
        if (pair.Key.ToLower() == "ulong")
        { return GetUnsignedInt64(pair.Value); }
        if (pair.Key.ToLower() == "bool")
        { return GetBool(pair.Value); }
        if (pair.Key.ToLower() == "ushort")
        { return GetUnignedInt16(pair.Value); }
        if (pair.Key.ToLower() == "uint")
        { return GetUnsignedInt32(pair.Value); }

Screaming to be a switch statement.

Second, these guys in every GetValues method:

Dictionary<string, string> propertyList = new Dictionary<string, string>(){

    /// <summary>
    /// 
    /// </summary>
    {"ushort", "AddressWidth"},

    /// <summary>
    /// 
    /// </summary>
    {"ushort", "Architecture"},};

a) are immutable once created, so it makes better sense to make them class-level variables. So they're initialized only once. b) should then be readonly. c) should be declared as IDictionary<string, string> so that you're developing against an interface rather than an implementation. This will need to be changed throughout method signatures, etc.

Third: GetUnignedInt16 really should be spelled correctly. GetUnsignedInt16.

Fourth: GetProperties() in the PropertyValue class does not "get properties". It gets a single property. It takes the Dictionary, but only returns the first property value (this is due to return being in the foreach loop. My guess is maybe you mean to add to a list or array?). Seems like a misnaming and a bug at the same time. Fix the code before the review.

Those are initial thoughts.

\$\endgroup\$
3
  • \$\begingroup\$ If GetProperties() is actually supposed to return all properties, simply writing it with yield return will do the trick. \$\endgroup\$
    – Bobson
    Mar 10, 2014 at 17:43
  • \$\begingroup\$ And with the required IEnumerable<object> return type. yield return methods must have a collection return type. \$\endgroup\$ Mar 10, 2014 at 17:53
  • \$\begingroup\$ A minor issue: You should use .ToUpperInvariant. It will probably never matter, but ToUpper is "better" than ToLower -- I don't have a link to the actual reason, but I believe it is really only an issue for some Unicode characters. And similarly using the Invariant culture mostly just avoids Code Analysis warnings, but they are there for a (most unlikely) reason. \$\endgroup\$
    – Mark Hurd
    Mar 31, 2014 at 2:35
3
\$\begingroup\$

You haven't been online in quite some time, but anyway ..

Review

  • You are not using the defaultValueIfEmpty from previous post. This means return (!enu.MoveNext() || enu.Current[propertyName] == null) ? null : enu.Current[propertyName]; can be shortened to return !enu.MoveNext() ? null : enu.Current[propertyName];.
  • Use a better key as String, prefer Type. You don't need any shenanigans with casing keys. Refactor Dictionary<string, string> propertyList to Dictionary<Type, string> propertyList.
  • public object GetValues() returns a single value. This seems more a bug than as designed.
  • ulong KiloBytesToMegaBytes there is a lot of misconception about these units. Check out Megabyte vs Mebibyte.
\$\endgroup\$
2
  • 1
    \$\begingroup\$ This mebibyte is a funny unit; and apparently nobody likes it :P \$\endgroup\$
    – t3chb0t
    Jul 8, 2019 at 18:15
  • 1
    \$\begingroup\$ It was an attempt at generalising units. But even my teacher in uni, who worked at IBM haven't heard about it :-p It is interesting though, that many different variations of formulas are used. \$\endgroup\$
    – dfhwze
    Jul 8, 2019 at 18:17

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.