Совершенствование конструкции эту программу.


Я пытаюсь разработать программу, которая будет читать XML-файл и на основе имени тега будет выполнять определенные проверки требований. Я чувствую, что есть, вероятно, лучший способ сделать это, и любые замечания или предложения приветствуются.

Спасибо.

public enum AccountCheckType
{
    UserIsMemberOfGroup,
    DomainUserExists,
    DomainGroupExists
}


public class AccountManager
{

    public bool DomainGroupExists(string groupName)
    {
        //Todo: Implement this
        return false;
    }

    public bool DomainUserExists(string userName)
    {
        //Todo: Implement this
        return false;
    }

    public bool UserIsMemberOfGroup(string userName,string groupName)
    {
        //Todo: Implement this
        return false;
    }
}


public class AccountRequirement:Requirement
{
    #region Declarations

    private readonly AccountManager manager = new AccountManager();
    private readonly AccountCheckType checkType;
    private string accountName;
    private string groupName;

    #endregion

    #region Constructor/Deconstructor

    /// <summary>
    /// Initializes a new instance of the <see cref="AccountRequirement"/> class.
    /// </summary>
    public AccountRequirement(string accountName,AccountCheckType checkType)
    {
        if(string.IsNullOrEmpty(accountName))
        {
            throw new ArgumentException("Account name cannot be null or empty.");
        }

        if(checkType==AccountCheckType.UserIsMemberOfGroup)
        {
            throw new ArgumentException("Checktype cannot be equal to UserIsMemberOfGroup");
        }

        this.accountName = accountName;
        this.checkType = checkType;
    }

    /// <summary>
    /// Initializes a new instance of the <see cref="AccountRequirement"/> class.
    /// </summary>
    /// <param name="userName">Name of the user.</param>
    /// <param name="groupName">Name of the group.</param>
    public AccountRequirement(string userName,string groupName)
    {
        if(string.IsNullOrEmpty(userName))
        {
            throw new ArgumentException("Username cannot be null or empty.");
        }

        if (string.IsNullOrEmpty(groupName))
        {
            throw new ArgumentException("Groupname cannot be null or empty.");
        }

        this.checkType = AccountCheckType.UserIsMemberOfGroup;
    }

    #endregion

    public override RequirementStatus PerformCheck()
    {
        switch (checkType)
        {
            case AccountCheckType.DomainGroupExists:
                CurrentStatus = manager.DomainGroupExists(groupName) ? OnPass : OnFail;
                return CurrentStatus;
            case AccountCheckType.DomainUserExists:
                CurrentStatus = manager.DomainUserExists(accountName) ? OnPass : OnFail;
                return CurrentStatus;
            case AccountCheckType.UserIsMemberOfGroup:
                CurrentStatus = manager.UserIsMemberOfGroup(accountName, groupName) ? OnPass : OnFail;
                return CurrentStatus;
            default:
                throw new Exception("Unknown checkType in account requirement");
        }
    }
}

public enum RegistryCheckType
{
    RegistryKeyExists,
    RegistryValueExists
}

public class RegistryRequirement:Requirement
{
    #region Declarations

    private RegistryManager manager = new RegistryManager();
    private RegistryCheckType checkType;
    private string key;
    private string subKey;
    private string value;

    #endregion

    #region Constructor/Deconstructor

    /// <summary>
    /// Initializes a new instance of the <see cref="RegistryRequirement"/> class.
    /// </summary>
    public RegistryRequirement(string key,string subKey)
    {
        if(string.IsNullOrEmpty(key))
        {
            throw new ArgumentException("Key cannot be null or empty.");
        }

        if (string.IsNullOrEmpty(subKey))
        {
            throw new ArgumentException("Subkey cannot be null or empty.");
        }

        this.key = key;
        this.subKey = subKey;
        this.checkType = RegistryCheckType.RegistryKeyExists;
    }

    public RegistryRequirement(string key, string subKey,string value)
    {
        if (string.IsNullOrEmpty(key))
        {
            throw new ArgumentException("Key cannot be null or empty.");
        }

        if (string.IsNullOrEmpty(subKey))
        {
            throw new ArgumentException("Subkey cannot be null or empty.");
        }

        if (string.IsNullOrEmpty(value))
        {
            throw new ArgumentException("Value cannot be null or empty.");
        }

        this.key = key;
        this.subKey = subKey;
        this.value = value;
        this.checkType = RegistryCheckType.RegistryValueExists;
    }

    #endregion


    public override RequirementStatus PerformCheck()
    {
        switch (checkType)
        {
            case RegistryCheckType.RegistryKeyExists:
                CurrentStatus = manager.RegistryKeyExists(key, subKey) ? OnPass : OnFail;
                return CurrentStatus;
            case RegistryCheckType.RegistryValueExists:
                CurrentStatus = manager.RegistryValueExists(key, subKey, value) ? OnPass : OnFail;
                return CurrentStatus;
            default:
                throw new Exception("Unknown checkType in registry requirement");
        }
    }
}

public abstract class Requirement
{
    #region Constructor/Deconstructor

    /// <summary>
    /// Initializes a new instance of the <see cref="Requirement"/> class.
    /// </summary>
    public Requirement()
    {
    }

    #endregion

    #region Properties

    public RequirementStatus OnPass { get; set; }
    public RequirementStatus OnFail { get; set; }
    public RequirementStatus CurrentStatus { get; protected set; }

    #endregion

    public abstract RequirementStatus PerformCheck();
}

public class RegistryManager
{
    #region Declarations



    #endregion

    #region Constructor/Deconstructor

    /// <summary>
    /// Initializes a new instance of the <see cref="RegistryManager"/> class.
    /// </summary>
    public RegistryManager()
    {
    }

    #endregion

    #region Properties



    #endregion

    public bool RegistryKeyExists(string key,string subKey)
    {
        //Todo: Implement this
        return true;
    }

    public bool RegistryValueExists(string key,string subKey,string value)
    {
        //Todo: Implement this
        return true;
    }
}

public class RequirementFileReader
{
    #region Declarations

    private XmlDocument xmlDocument=new XmlDocument();
    private string filePath;

    #endregion

    #region Constructor/Deconstructor

    /// <summary>
    /// Initializes a new instance of the <see cref="RequirementFileReader"/> class.
    /// </summary>
    public RequirementFileReader(string filePath)
    {
        if(string.IsNullOrEmpty(filePath))
        {
            throw new ArgumentException("File path cannot be null or empty.");
        }

        this.filePath = filePath;
    }

    #endregion

    #region Properties



    #endregion

    public IEnumerable<Requirement> GetRequirements()
    {
        var requirements=new List<Requirement>();
        xmlDocument.Load(filePath);
        var rootNode = xmlDocument.DocumentElement;
        if(rootNode==null)
        {
            throw new Exception("Not a valid xml file.");
        }
        foreach (XmlNode node in rootNode.ChildNodes)
        {
            var onPass = (RequirementStatus) Enum.Parse(typeof (RequirementStatus), node.Attributes["onpass"].Value);
            var onFail = (RequirementStatus) Enum.Parse(typeof (RequirementStatus), node.Attributes["onfail"].Value);

            Requirement req = null;
            switch (node.Name)
            {
                case "UserExists":
                    var user=node.Attributes["username"].Value;
                    req = new AccountRequirement(user, AccountCheckType.DomainUserExists)
                              {OnPass = onPass, OnFail = onFail};
                    break;
                case "GroupExists":
                    var group = node.Attributes["groupname"].Value;
                    req = new AccountRequirement(group, AccountCheckType.DomainGroupExists) { OnPass = onPass, OnFail = onFail };
                    break;
                case "RegKeyExists":
                    var key=node.Attributes["key"].Value;
                    var subkey=node.Attributes["subkey"].Value;
                    req = new RegistryRequirement(key, subkey);
                    break;
                case "RegValueExists":
                    var key1 = node.Attributes["key"].Value;
                    var subkey1=node.Attributes["subkey"].Value;
                    var value1=node.Attributes["value"].Value;
                    req = new RegistryRequirement(key1, subkey1, value1);
                    break;
                default:
                    throw new Exception("Invalid xml tag found.");
            }
            requirements.Add(req);
        }
        return requirements;
    }
}


481
5
задан 28 мая 2011 в 07:05 Источник Поделиться
Комментарии
3 ответа

На вещь, которая меня беспокоит, это количество коммутатор отчетность у вас в коде. Некоторые ОО практиков рассматривают это как код запах.

Одна возможность состоит в том, чтобы заполнить словарь:

public delegate Requirement AllDelegates(XmlNode node);
Dictionary<string,delegate> typeDictionary;

typeDictionary.Add("UserExists", RequirementDelegates.UserDelegate);
typeDictionary.Add("GroupExists", RequirementDelegates.GroupDelegate);
typeDictionary.Add("RegKeyExists", RequirementDelegates.RegKeyDelegate);
typeDictionary.Add("RegValueExists", RequirementDelegates.RegValueDelegate);

где, например, UserDelegate является статическим членом RequirementDelegates класс:

class RequirementDelegates
{
public static Requirement UserDelegate(XmlNode node,
RequirementStatus onPass,
RequirementStatus onFail )
{
var user = node.Attributes["username"].Value;
var req = new AccountRequirement(user, AccountCheckType.DomainUserExists)
{OnPass = onPass, OnFail = onFail};
return req;
}
}

Потом ваш большой рубильник заявление в конце просто становится:

AllDelegates theDelegate = typeDictionary[node.Name];
var req = theDelegate(node, onPass, onFail);

Делегаты все еще находятся в одном месте, но логика яснее в GetRequirements способ.

4
ответ дан 28 мая 2011 в 09:05 Источник Поделиться

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

Если все вы хотите, чтобы вытащить определенные биты информации из определенных элементов, то я бы определенно использовать XPath для достижения этой цели. Вы можете легко извлекать данные из элементы, необходимые для инициализации других классов, без всякого хлама в цикле foreach и переключатель в GetRequirements способ.

Кроме того, вы можете создать xsd (если не уже) для XML вы потребляете, используя XSD.exeи затем генерировать C# классы из xsd.Вы, вероятно, хотите изменить xsd-файле, так что различные элементы требуют они содержат текст или атрибуты. Затем вы могли бы использовать xsd, чтобы быстро проверить XML-документ и использовать классы C#, чтобы serialise XML в объекты, которые затем можно использовать, чтобы сделать дальнейшие проверки и инициализации другими объектами.

1
ответ дан 31 мая 2011 в 10:05 Источник Поделиться

Вы, кажется мне, хороший дизайн, было бы здорово, а также повысить читаемость программы, если вы измените имя функции, которую выполняют проверку.

Они должны начать с это

например ---> ваше название функции DomainGroupExists может быть изменение IsDomainGroupExists ... это может применяться ко всем вашим проверкам функций.

0
ответ дан 28 мая 2011 в 09:05 Источник Поделиться