c# Инспекция кода - Формирование строки с параметрами компьютера, используя Task

Всем привет. Можно ли как-то чуть красивее оформить вызов задач (может даже быстрее, но я, пока что, не знаю, как это ещё быстрее сделать...)?

Суть задачи: Есть несколько методов, каждый из который выполняется за N времени (какой-то быстрее, какой-то чуть медленнее). Для оптимизации кода было решено выполнить данные методы в разных потоках. Таким образом получился следующий код:

Task task1 = new Task(CPU_ID);
task1.Start();
Task task2 = new Task(BIOS_ID);
task2.Start();
Task task3 = new Task(BaseBoardID);
task3.Start();
Task task4 = new Task(DiskDriveID);
task4.Start();
Task task5 = new Task(VideoControllerID);
task5.Start();
Task.WaitAll(task1, task2, task3, task4, task4, task5);
task1.Dispose();
task2.Dispose();
task3.Dispose();
task4.Dispose();
task5.Dispose();
    
HWID = HWIDClass.CPU + HWIDClass.BIOS + HWIDClass.BaseBoard + HWIDClass.DiskDrive + HWIDClass.VideoController;

Мне не очень нравится визуально такой кусок кода, где куча повторений. Можно ли это как-то свернуть (может в цикл или как-нибудь так)?

P.S. Результат работы каждого метода - передача в 'HWIDClass' переменной типа string

UPD:

using System.Collections.Generic;
using System.Management;
using System.Threading;
using System.Threading.Tasks;

namespace HWIDVerification
{
    public class CerialNumberAsync
    {
        private static class HWIDClass
        {
            public static string CPU { get; set; }
            public static string BIOS { get; set; }
            public static string BaseBoard { get; set; }
            public static string DiskDrive { get; set; }
            public static string VideoController { get; set; }
        }

        public string HWID { get; private set; }
        public string HWIDHash { get; private set; }

        public CerialNumberAsync()
        {
            Value();
        }

        private void Value()
        {
            if (string.IsNullOrEmpty(string.Empty))
            {
                Task task1 = new Task(CPU_ID);
                task1.Start();
                Task task2 = new Task(BIOS_ID);
                task2.Start();
                Task task3 = new Task(BaseBoardID);
                task3.Start();
                Task task4 = new Task(DiskDriveID);
                task4.Start();
                Task task5 = new Task(VideoControllerID);
                task5.Start();
                Task.WaitAll(task1, task2, task3, task4, task4, task5);
                task1.Dispose();
                task2.Dispose();
                task3.Dispose();
                task4.Dispose();
                task5.Dispose();

                HWID = HWIDClass.CPU + HWIDClass.BIOS + HWIDClass.BaseBoard + HWIDClass.DiskDrive + HWIDClass.VideoController;

                HWIDHash = HashClass.GetHash(HWID);
            }
        }
        #region Original Device ID Getting Code
        private static string identifier (string wmiClass, string wmiProperty, string wmiMustBeTrue)
        {
            string result = "";
            ManagementObjectCollection moc = new ManagementClass(wmiClass).GetInstances();
            foreach (ManagementObject mo in moc)
            {
                if (mo[wmiMustBeTrue].ToString() == "True")
                {
                    if (result == "")
                    {
                        try
                        {
                            result = mo[wmiProperty].ToString();
                            break;
                        }
                        catch
                        {
                        }
                    }
                }
            }
            return result;
        }
        private static string identifier(string wmiClass, string wmiProperty)
        {
            string result = "";
            ManagementClass mc = new ManagementClass(wmiClass);
            ManagementObjectCollection moc = mc.GetInstances();
            foreach (ManagementObject mo in moc)
            {
                if (result == "")
                {
                    try
                    {
                        result = mo[wmiProperty].ToString();
                        break;
                    }
                    catch
                    {
                    }
                }
            }
            return result;
        }

        private static string TryGetString(string wmiClass, string wmiProperty)
        {
            try 
            {
                string result = $"{wmiProperty}: " + identifier(wmiClass, wmiProperty);
                return result + "\n";
            }
            catch { return null; }
        }

        private static void CPU_ID()
        {
            string result = TryGetString("Win32_Processor", "ProcessorId");
            result += TryGetString("Win32_Processor", "Name");
            result += TryGetString("Win32_Processor", "Manufacturer");
            result += TryGetString("Win32_Processor", "MaxClockSpeed");
            HWIDClass.CPU = "CPU >> " + result;
        }
        private static void BIOS_ID()
        {
            string result = TryGetString("Win32_BIOS", "SerialNumber");
            result += TryGetString("Win32_BIOS", "SMBIOSBIOSVersion");
            result += TryGetString("Win32_BIOS", "Manufacturer");
            result += TryGetString("Win32_BIOS", "ReleaseDate");
            result += TryGetString("Win32_BIOS", "Version");
            HWIDClass.BIOS = "BIOS >> " + result;
        }
        private static void BaseBoardID()
        {
            string result = TryGetString("Win32_BaseBoard", "SerialNumber");
            result += TryGetString("Win32_BaseBoard", "Name");
            result += TryGetString("Win32_BaseBoard", "Manufacturer");
            HWIDClass.BaseBoard = "BaseBoard >> " + result;
        }
        private static void DiskDriveID()
        {
            string result = TryGetString("Win32_DiskDrive", "SerialNumber");
            result += TryGetString("Win32_DiskDrive", "TotalHeads");
            result += TryGetString("Win32_DiskDrive", "Signature");
            result += TryGetString("Win32_DiskDrive", "Manufacturer");
            result += TryGetString("Win32_DiskDrive", "Model");
            HWIDClass.DiskDrive = "DiskDrive >> " + result;
        }
        private static void VideoControllerID()
        {
            string result = TryGetString("Win32_VideoController", "PNPDeviceID");
            result += TryGetString("Win32_VideoController", "Name");
            result += TryGetString("Win32_VideoController", "AdapterRAM");
            HWIDClass.VideoController = "VideoController >> " + result;
        }
        private static string MAC_ID()
        {
            try { return identifier("Win32_NetworkAdapterConfiguration", "MACAddress", "IPEnabled"); }
            catch { return null; }
        }
        #endregion
    }
}

Взаимодействие с классом такое:

Создаём экземпляр класса: CerialNumberAsync cerialHash = new CerialNumberAsync(); Далее, просто получаем из данного объекта строковую переменную HWIDHash: cerialHash.HWIDHash


Ответы (1 шт):

Автор решения: aepot

Первое, что здесь не так - это использование конструкторов Task. Не используйте никогда конструктор Task, это устаревшее наследие, существующее для совместимости с древними дотнетами.

Конструкцию

Task task1 = new Task(CPU_ID);
task1.Start();

Можно и нужно преобразовать в:

Task task1 = Task.Run(CPU_ID);

Второе, что здесь не так, это баг, опечатка task4, task4.

Третье заключается в вызове task1.Dispose();. Task имплементирует IDisposable, а это значит, что внутри него есть IDisposable либо неуправляемые компоненты, и они есть. И этот компонент task1.WaitHandle, и создается он только в том случае, если вы хотя-бы один раз к нему обратились. В противном случае, а это 99,9% случаев использования, Task диспозить не нужно.

Вот и получается -10 строк.

Task task1 = Task.Run(CPU_ID);
Task task2 = Task.Run(BIOS_ID);
Task task3 = Task.Run(BaseBoardID);
Task task4 = Task.Run(DiskDriveID);
Task task5 = Task.Run(VideoControllerID);
Task.WaitAll(task1, task2, task3, task4, task5);
    
HWID = HWIDClass.CPU + HWIDClass.BIOS + HWIDClass.BaseBoard + HWIDClass.DiskDrive + HWIDClass.VideoController;

И это еще только начало.

Далее, именования CerialNumberAsync, здесь Cerial наверное Serial и почему Async? Здесь нет даже намека на асинхронность и суффикс Async дописывают обычно методам, а не классам. Переименуйте например в HWIDExtractor. Будет понятно, что он "извлекает" HWID.

Имена методов. По конвенции Microsoft предложено аббревиатуры длинной более 2 символов писать просто с большой буквы, а не большими буквами. Например BIOS => Bios, сами же названия методов писать используя Camel Case, без подчеркиваний.

Разделяемые ресурсы. В многопоточной среде надо избегать использования разделяемых ресурсов настолько, насколько это возможно. Пусть даже у вас здесь выглядит безопасно, лучше убрать общий класс HWIDClass, без него здесь очень просто можно обойтись.

Чтобы обойтись без лишней структуры данных, можно результат работы возвращать из методов через возвращаемое значение, а не через разделяемый ресурс.

Кстати, if (string.IsNullOrEmpty(string.Empty)) это всегда true, наверное вы имели в виду if (string.IsNullOrEmpty(HWID))? И это проверка здесь бессмысленная, потому что метод приватный и вызывается только из конструктора, то есть ровно один раз, не больше.

Применим к вызываемым методам предложенные изменения.

private static string CpuID()
{
    string result = TryGetString("Win32_Processor", "ProcessorId");
    result += TryGetString("Win32_Processor", "Name");
    result += TryGetString("Win32_Processor", "Manufacturer");
    result += TryGetString("Win32_Processor", "MaxClockSpeed");
    return "CPU >> " + result;
}

private static string BiosID()
{
    string result = TryGetString("Win32_BIOS", "SerialNumber");
    result += TryGetString("Win32_BIOS", "SMBIOSBIOSVersion");
    result += TryGetString("Win32_BIOS", "Manufacturer");
    result += TryGetString("Win32_BIOS", "ReleaseDate");
    result += TryGetString("Win32_BIOS", "Version");
    return "BIOS >> " + result;
}

private static string BaseBoardID()
{
    string result = TryGetString("Win32_BaseBoard", "SerialNumber");
    result += TryGetString("Win32_BaseBoard", "Name");
    result += TryGetString("Win32_BaseBoard", "Manufacturer");
    return "BaseBoard >> " + result;
}

private static string DiskDriveID()
{
    string result = TryGetString("Win32_DiskDrive", "SerialNumber");
    result += TryGetString("Win32_DiskDrive", "TotalHeads");
    result += TryGetString("Win32_DiskDrive", "Signature");
    result += TryGetString("Win32_DiskDrive", "Manufacturer");
    result += TryGetString("Win32_DiskDrive", "Model");
    return "DiskDrive >> " + result;
}

private static string VideoControllerID()
{
    string result = TryGetString("Win32_VideoController", "PNPDeviceID");
    result += TryGetString("Win32_VideoController", "Name");
    result += TryGetString("Win32_VideoController", "AdapterRAM");
    return "VideoController >> " + result;
}

Далее метод Value, почему Value? GetHWID было бы лучше, и так же через возвращаемое значение.

Далее, так как есть теперь 5 возвращаемых строк, можно пробросить их через таски, объединю методы в массив делегатов. Тогда на помощь придет всемогущий Linq.

private string GetHWID()
{
    Func<string>[] methods = new[] { CpuID, BiosID, BaseBoardID, DiskDriveID, VideoControllerID };
    return string.Concat(Task.WhenAll(methods.Select(Task.Run)).Result);
}

Теперь оно будет выглядеть так, что свойства инициализируются в конструкторе, как и должны. Код легче читать.

public HWIDExtractor()
{
    HWID = GetHWID();
    HWIDHash = HashClass.GetHash(HWID);
}

Кстати, почему если я BIOS пишу Bios, а HWID большими буквами, а потому что BIOS - это аббревиатура 4 слов, а HWID не аббревиатура, а сокращение двух слов, или 2 двухбуквенные аббревиатуры, и если бы я подчинялся правилам майкрософта, то выглядело бы так HwId - Hardware Identifier. Поэтому я каждое сокращаю до двух букв и получается, что 2 буквы пишем большими буквами - HW и ID.

И вишенка на торте, теперь, когда свойства присваиваются только в конструкторе - можно выкинуть сеттеры.

public string HWID { get; }
public string HWIDHash { get; }

Напоследок увидел еще странный метод

private static string TryGetString(string wmiClass, string wmiProperty)
{
    try 
    {
        string result = $"{wmiProperty}: " + identifier(wmiClass, wmiProperty);
        return result + "\n";
    }
    catch { return null; }
}

Я не уверен, что identifier вообще может бросить исключение, есть ли смысл заворачивать его в try-catch? Но если не рисковать, и оставить как есть, то гораздо аккуратнее получится вот так.

return $"{wmiProperty}: {identifier(wmiClass, wmiProperty)}\n";

А то намешано конкатенации с интерполяцией. Старайтесь писать однотиным образом.

→ Ссылка