Метод рефакторинга для уменьшения количества операторов switch — C#

Переключатели — это запах кода, который говорит, что ваш код не объектно-ориентированный. В этом случае у вас могут быть разные типы SearchModel с методом search, реализованным по-разному для каждого типа.

Я согласен с ТКК

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

Первый оператор switch установит sqlcommand и параметры, а второй останется неизменным в цикле while.

Объявите их в верхней части вашего метода:

`SqlConnection conn = new SqlConnection("connection string");
 SqlCommand cmd = AdoBase.GetSqlCommand("", conn);`

Измените первый оператор switch, чтобы добавить параметры в команду sql:

switch (model.Attribute)
{
    case "Users":
        sqlCommand = "GeneralUserSearch";
        if (!string.IsNullOrWhiteSpace(model.Name))
        {
            cmd.Parameters.AddWithValue("Name", model.Name);
        }

        if (!string.IsNullOrWhiteSpace(model.Name))
        {
            cmd.Parameters.AddWithValue("Username", model.Username);
        }

        break;
    case "Favorites":
        sqlCommand = "UserFavorites";
        cmd.Parameters.AddWithValue("Favorites", model.Favorites);
        break;
    case "Email":
        sqlCommand = "EmailSearch";
        cmd.Parameters.AddWithValue("Email", model.Email);
        break;
}

Удалите этот блок кода:

switch (model.Attribute)
{
    case "Users":
        if (!string.IsNullOrWhiteSpace(model.Name)) {
            cmd.Parameters.AddWithValue("Name", model.Name);
        }
        if (!string.IsNullOrWhiteSpace(model.Name)) {
            cmd.Parameters.AddWithValue("Username", model.Username);
        }                                                           
        break;
    case "Favorites":
        cmd.Parameters.AddWithValue("Favorites", model.Favorites);
        break;
    case "Email":
        cmd.Parameters.AddWithValue("Email", model.Email);
        break;                                             
}

И добавьте, наконец, после вашего улова, чтобы очистить объекты соединения и команды:

 finally
 {
    conn.Dispose();
    cmd.Dispose();
 }

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

public static SearchModel GetSearchResults(SearchModel model)
{
    List<ResultModel> results = new List<ResultModel>();
    SqlCommand cmd = new SqlCommand(); // only for compile for finnaly dispose
    try
    {
        using (SqlConnection conn = new SqlConnection("connection string"))
        {
            conn.Open();
            string sqlCommand = string.Empty;

            switch (model.Attribute)
            {
                case "Users":
                    cmd = AdoBase.GetSqlCommand("GeneralUserSearch", conn);
                    if (!string.IsNullOrWhiteSpace(model.Name)) {
                        cmd.Parameters.AddWithValue("Name", model.Name);
                    }
                    if (!string.IsNullOrWhiteSpace(model.Name)) { // redundant if or mistake, and there should be model.Username
                        cmd.Parameters.AddWithValue("Username", model.Username);
                    }
                    break;
                case "Favorites":
                    cmd = AdoBase.GetSqlCommand("UserFavorites", conn);
                    cmd.Parameters.AddWithValue("Favorites", model.Favorites);
                    break;
                case "Email":
                    cmd = AdoBase.GetSqlCommand("EmailSearch", conn);
                    cmd.Parameters.AddWithValue("Email", model.Email);
                    break;
            }
            SimpleMethod(results, cmd, model);
        }
    }
    catch (Exception ex)
    {
        return ex;
    }
    finally
    {
        cmd.Dispose();
    }
    return model;
}

Простой метод:

public static void SimpleMethod(List<ResultModel> results, SqlCommand cmd, SearchModel model)
{
    using (SqlDataReader reader = cmd.ExecuteReader())
    {
        if (reader.HasRows)
        {
            while (reader.Read())
            {
                ResultModel result = new ResultModel();

                switch (model.Attribute)
                {
                    case "Users":
                        result.Users.Add(reader["User"]);
                        break;
                    case "Favorites":
                        result.User = reader["User"];
                        result.Favorites = reader["Favorites"];
                        break;
                    case "Email":
                        result.User = reader["User"];
                        result.Email = reader["Email"];
                        break;
                }
                results.Add(result);
            }
        }
    }
}

Другие ответы полны анти-шаблонов. Необходимость в операторах switch (особенно повторяющихся так часто) кажется возможностью двигаться в направлении большего ООП.

Я не рефакторил все это, а просто чтобы дать вам представление.

public class SearchHelper
{
    //why does this need to return the model at all? the model isn't altered 
    //and would already be in scope for whatever is calling this method
    public static void GetSearchResults(SearchModel model)
    {
        List<ResultModel> results = new List<ResultModel>();

        try
        {
            using (SqlConnection conn = new SqlConnection("connection string"))
            {
                conn.Open();

                using (SqlCommand cmd = AdoBase.GetSqlCommand(model.SqlCommandName, conn))
                {
                    //this will mutate the object, so you don't need a return type. I'd suggest refactoring this further.
                    model.BuildSqlCommand(cmd);

                    using (SqlDataReader reader = cmd.ExecuteReader())
                    {
                        //your code sample wasn't returning this, but maybe you intended to?
                        BuildResultSet(reader);
                    }

                }
            }
        }
        catch (Exception ex)
        {
            throw ex;
        }
    }

    private static IEnumerable<ResultModel> BuildResultSet(SqlDataReader reader)
    {
        var results = new List<ResultModel>();

        if (!reader.HasRows) { return results; }

        while (reader.Read())
        {
            ResultModel result = new ResultModel();

            //  ...the result composition would need to be refactored in a similar way as well
            results.Add(result);
        }
        return results;
    }
}

public abstract class SearchModel
{
    public string SqlCommandName { get; private set; }

    private SearchModel() { }

    protected SearchModel(string sqlCommandName)
    {
        SqlCommandName = sqlCommandName;
    }

    public abstract void BuildSqlCommand(SqlCommand command);
}

public class UserSearchModel : SearchModel
{
    public string Name { get; set; }

    public string Username { get; set; }

    public UserSearchModel() : base("GeneralUserSearch")
    {
    }

    //warning...this mutates the input parameter
    public override void BuildSqlCommand(SqlCommand command)
    {
        if (!string.IsNullOrWhiteSpace(Name))
        {
            command.Parameters.AddWithValue(nameof(Name), Name);
        }
        if (!string.IsNullOrWhiteSpace(Username))
        {
            command.Parameters.AddWithValue(nameof(Username), Username);
        }
    }
}

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

Тем не менее, вы можете видеть, насколько это субъективно и может быть неуместным для SO. Кажется, здесь проблема с фундаментальным дизайном.