Нарушает ли моя функция SRP или другие рекомендации?

У меня есть следующий фрагмент кода, функция, которая получает список объектов (я называю это объектами Y) и вызывает другой метод для преобразования его в объекты X.

public List<X> GetXObjects()
{
    var yObjects= GetYObjects();
    var transformer = new Transformer();
    var xObjects = transformer.Transform(yObjects);
    return xObjects;
}

Текущая реализация работает, однако я чувствую, что моя функция может нарушать SRP или другие рекомендации. Можно ли отрефакторить код, чтобы он стал лучше?


c# refactoring single-responsibility-principle
person Anthony Steven    schedule 17.07.2017    source источник

Ответы (3)


arrow_upward
3
arrow_downward

Я бы назвал метод TransformObjects и добавил преобразователь в качестве параметра, чтобы можно было использовать разные преобразователи, а преобразователь не инициализировался каждый раз, когда вы вызываете метод. То же самое для yObjects. Таким образом, единственной обязанностью метода является преобразование ваших объектов.

public List<X> TransformObjects(List<Y> yObjects, Transformer transformer)
{
    var xObjects = transformer.Transform(yObjects);
    return xObjects;
}

вызов метода:

var yObjects= _crmWrapper.GetActivitiesByOpportunities();
var transformer = new Transformer();
myTransformer.TransformObjects(yObjects, transformer);

Если вы создаете интерфейс ITransformer, было бы проще улучшить ваш код, заменив преобразователь.

person Kevin Wallis    schedule 17.07.2017
comment
Хороший ответ, но если вы пойдете по этому пути, зачем изобретать велосипед? Нет необходимости в TransformObjects, Enumerable.Select уже выполняет эту работу (подробности см. В моем ответе). - person InBetween; 17.07.2017
comment
@InBetween очень хороший совет! Я хотел объяснить, как вообще можно достичь SRP -› не используя Enumerable.Select. - person Kevin Wallis; 17.07.2017

arrow_upward
2
arrow_downward

Мои два цента...

Я бы сделал этот код более общим за небольшие деньги. Продолжая тему хорошего ответа Кевина Уоллиса, почему бы просто не удалить этот надоедливый преобразователь? Подойдет все, что знает, как преобразовать Y в X, верно?

И зачем принимать и возвращать List? Пусть потребитель решает, хочет ли он жадно потреблять перечисление или нет, не принимайте решение за него, если в этом нет необходимости.

public IEnumerable<TResult> TransformObjects<TResult, TSource>(
    IEnumerable<TSource> source, Func<TSource, TResult> transformer)
    => source.Select(transformer);

И теперь вдруг вы понимаете; Привет! это просто обычный старый добрый Enumerable.Select, не так ли? Тогда зачем изобретать велосипед?

var xObjects = GetYObjects().Select(transformer.Transform);

Итак, ваш код, наконец, Enumerable.Select! Отлично, здесь нет ошибок, и мне нужно протестировать на один метод меньше, идем дальше ;).

person InBetween    schedule 17.07.2017

arrow_upward
1
arrow_downward

Ваша функция выполняет следующие задачи

  • нагрузки
  • создать экземпляр трансформатора
  • трансформировать загруженные объекты

Загрузку действий можно удалить, введя ее в качестве аргумента.

public List<X> GetXObjects(List<Y> yObjects)
{
    var transformer = new Transformer();
    return transformer.Transform(yObjects);
}

Тогда создание трансформатора можно вынести и наружу

public List<X> GetXObjects(Trnasformer transformer, List<Y> yObjects)
{
    return transformer.Transform(yObjects);
}

Итак, теперь где-то в коде вы называете это так

var yObjects= _crmWrapper.GetActivitiesByOpportunities();
var transformer = new Transformer();
var xObjects = GetXObjects(yObjects, transformer);

Но хиты означают, что везде, где вам нужно загрузить XObjects, вам нужно знать, как загрузить YObjects и как создать Transformer

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

public class XObjectsProvider
{

}

Этот класс должен загрузить YObjects и преобразовать их в XObject. Фактическая реализация загрузки и создания экземпляра Transform не входит в обязанности этого класса, поэтому мы вводим их как зависимости.

public class XObjectsProvider
{
    private Loader _loader;
    private Transformer _transformer;

    public XObjectsProvider(Loader loader, Transformer transformer)
    {
        _loader = loader;
        _transformer = transformer;
    }

    public XObjects Get()
    {
        var yObjects = _loader.GetYObjects();
        return transformer.Transform(yObjects);
    }
}

Loader и Transformer могут быть представлены как абстракции, которые дают вам возможность изменять их без изменения кода XObjectProvider - следуя "принципу открытия/закрытия". А у XObjectProvider есть только одна причина для изменения - когда для получения XObjects нужно загрузить/предоставить что-то другое.

person Fabio    schedule 17.07.2017