Что является хорошей практикой при написании метода, модифицирующего коллекцию c # - PullRequest
Новая
       1

Что является хорошей практикой при написании метода, модифицирующего коллекцию c #

2 голосов
/ 18 февраля 2011

Я рефакторинг некоторого кода, и я написал метод, который изменяет Словарь и возвращает его. Это лучше, чем использовать параметр out ? Я действительно не хочу создавать метод расширения в этом случае, потому что он добавил бы метод в класс Dictionary , который является излишним для того, для чего он используется. Пожалуйста, не указывайте, что я не должен использовать динамический sql, это еще один этап рефакторинга, который в настоящее время должен быть отложен. 

private static Dictionary<int, string>
            FindMatches(Dictionary<int, string> records,
                        string queryFormat,
                        string region,
                        string type,
                        string label)
{
    var query = string.Format(queryFormat, SqlSvrName, SqlDbName, SqlSchemaName,
                                                             region, type, label);
    using (var dr = DataRepository.Provider.ExecuteReader(CommandType.Text, query))
    {
        if (dr != null && !dr.IsClosed)
        {
            while (dr.Read())
            {
                var assetID = (int)dr.GetDouble(0);
                if (!records.ContainsKey(assetID))
                    records[assetID] = dr.GetString(1);
            }
        }
    }
    return records;
}

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

Edit2: просто чтобы продолжить, я удалил параметр records и вместо этого возвратил список KeyValuePair из FindMatches . Я получаю List<KeyValuePair<int, string>>, который преобразую в словарь с помощью: 

records
    .GroupBy(rec => rec.Key)
    .ToDictionary(grp => grp.Key, grp => grp.First().Value);

Ответы [ 5 ]

9 голосов
/ 18 февраля 2011

Почему ваш метод вообще должен изменять существующий словарь?Кажется, он не использует существующие ключи / значения, поэтому этот метод просто возвращает новый Dictionary<string, int>: 

private static Dictionary<int, string>
            FindMatches(string queryFormat,
                        string region,
                        string type,
                        string label)
{
    var records = new Dictionary<int, string>();
    var query = string.Format(queryFormat, SqlSvrName, SqlDbName,
                              SqlSchemaName, region, type, label);
    using (var dr = DataRepository.Provider.ExecuteReader(CommandType.Text,
                                                          query))
    {
        if (dr != null && !dr.IsClosed)
        {
            while (dr.Read())
            {
                var assetID = (int)dr.GetDouble(0);
                // If each assetID in the database will be distinct, you 
                // don't need the "if" here, because you know the dictionary
                // is empty to start with
                if (!records.ContainsKey(assetID))
                {
                    records[assetID] = dr.GetString(1);
                }
            }
        }
    }
    return records;
}

Вы можете также написать отдельный метод для объединения двух словарей определенным образом - или вернуть новый словарь, который является результатом объединения двух существующих. Разделите две проблемы.

4 голосов
/ 18 февраля 2011

Поскольку вы на самом деле не используете словарь в методе (кроме его заполнения), я бы удалил его из входных параметров и просто возвратил его.

Если по какой-то причине вам нужно передать словарь в качестве параметра, я бы вернул void и переименовал метод, чтобы убедиться, что он имеет побочные эффекты для входных параметров - например, 

void PopulateRecordsWithMatches(Dictionary<int, string> records...)
2 голосов
/ 18 февраля 2011

Я считаю, что ссылки на все сложные объекты в C # передаются по ссылке. Поэтому, если вы изменяете словарь по его ссылке внутри метода, вам не нужно возвращать его или указывать ключевое слово out / ref. Он будет изменен за пределами области действия метода.

0 голосов
/ 18 февраля 2011

Параметр out будет означать, что вы передадите пустой словарь. Это не имеет смысла. Вы, вероятно, имеете в виду параметр ref.

Но, как уже говорил Кон, вы можете изменить содержимое объекта внутри метода.

0 голосов
/ 18 февраля 2011

Поскольку вы изменяете словарь на месте, я думаю, что лучше сделать это явным, взяв его в качестве параметра ref: 

private static void
            FindMatches(ref Dictionary<int, string> records,
                        string queryFormat,
                        string region,
                        string type,
                        string label)
{
    var query = string.Format(queryFormat, SqlSvrName, SqlDbName, SqlSchemaName,
                                                             region, type, label);
    using (var dr = DataRepository.Provider.ExecuteReader(CommandType.Text, query))
    {
        if (dr != null && !dr.IsClosed)
        {
            while (dr.Read())
            {
                var assetID = (int)dr.GetDouble(0);
                if (!records.ContainsKey(assetID))
                    records[assetID] = dr.GetString(1);
            }
        }
    }
}

Возвращение или использование его в качестве выходного параметра подразумевает, что вы создаете новый словарь, а это не так. С другой стороны, явное указание в качестве параметра ref явного признака того, что вы намереваетесь изменить словарь как побочный эффект.

...