Refactoring: Check-Funktion

Original Code

private bool CheckAddNamespaceReservationParamters()
{
    var isUserParamSet = !IsNullOrEmpty(m_User);
    var isUrlParamSet = !IsNullOrEmpty(m_Url);
    var isUrlPathParamSet = !IsNullOrEmpty(m_UrlPath);
    if (m_IsAddNamespaceReservationOption)
    {
        if (isUrlParamSet && isUserParamSet && isUrlPathParamSet)
        {
            var isValid = validator.ValidateUrl(m_Url) && validator.ValidateUser(m_User)
                && validator.ValidateUrlPath(m_UrlPath) && validator.ValidatePort(m_Port);
            if (isValid)
            {
                m_Conf.Option = Configuration.ExecutionOption.AddNamespace;
                m_Conf.User = m_User;
                m_Conf.Url = m_Url;
                m_Conf.Port = m_Port;
                m_Conf.UrlPath = TrimApostrophes(m_UrlPath);
            }
            return isValid;
        }
    }
    return false;
}
        
private static string TrimApostrophes(string path)
{
    return path.Trim(Convert.ToChar("'"));
}

private static bool IsNullOrEmpty(string s)
{
    if (s == null)
    {
        return true;
    }

    if (s.Length > 0)
    {
        return true;
    }

    return false;
}

Refactoring Code 

private bool CheckAddNamespaceReservationParamters()
{
    if (String.IsNullOrEmpty(m_User)) return false; 
    if (String.IsNullOrEmpty(m_Url)) return false; 
    if (String.IsNullOrEmpty(m_UrlPath)) return false; 
    if (!m_IsRemoveNamespaceReservation) return false;

    if (validator.ValidateUrl(m_Url) && validator.ValidateUser(m_User) &&
        validator.ValidateUrlPath(m_UrlPath) && validator.ValidatePort(m_Port))
    {
        m_Conf.Option = Configuration.ExecutionOption.AddNamespace; 
        m_Conf.User = m_User; 
        m_Conf.Url = m_Url; 
        m_Conf.Port = m_Port; 
        m_Conf.UrlPath = m_UrlPath.Trim(Convert.ToChar("'")); 
        return true;
    } 
            
    return false;
}

Bemerkungen

  • Es macht keinen Sinn "IsNullOrEmpty" noch einmal neu zu implementieren(siehe DRY oder KISS).
  • Tiefe Verschachtelungen ( z.b. if{ if{ if{..}}}) sind schwerer zu lesen und Fehleranfälliger.
  • Es ist super Code wieder zu Verwenden und nicht einfach zu duplizieren aber bei so trivialen Funktionen wie Trim ist es eher irritierend und überflüssig (da die Funktion eh nur aus dem .Trim() Aufruf besteht).
  • Wie im Original schon zu sehen empfiehlt es sich eingangs variablen oder Bedingungen die zum Abbruch führen direkt am Anfang und als Block zu schreiben. Wenn Sie das konsequent bei jeder Funktion machen, dann erhöht sich dadurch die Lesbarkeit enorm da Sie diese Blöcke leichter überspringen können und direkt zu den interessanten Stellen gelangen.