Является ли это запахом кода, если вы часто создаете объект просто для вызова метода?

14

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

SomeDataAdapter sda = new SomeDataAdapter();
sda.UpdateData(DataTable updateData);

И тогда sda больше никогда не используется.

Это запах кода, который указывает, что эти методы должны быть статическими методами класса?

Шейн Веалти
источник
Эти классы реализуют какие-либо интерфейсы?
Reactgular
7
На следующий день после того, как вы выполните рефакторинг для статических методов, вы захотите запустить модульные тесты с фиктивной версией вашего адаптера данных.
Майк
22
@Mike: Зачем вам когда-нибудь издеваться над статическим методом? Я ужасно устал от этого ложного аргумента о том, что статические методы не поддаются проверке. Если ваши статические методы поддерживают состояние или создают побочные эффекты, это ваша вина, а не методология тестирования.
Роберт Харви
9
На мой взгляд, это языковой запах, показывающий слабость анализа «все должно быть классом».
Бен
7
@RobertHarvey: вам, возможно, понадобится смоделировать статический метод по той же причине, по которой вы издеваетесь над любым другим методом: его вызов во время модульного тестирования обходится слишком дорого.
Кевин Клайн

Ответы:

1

Я бы посчитал это запахом архитектуры в том смысле, что UpdateData, вероятно, должен принадлежать к классу «сервис».

Где данные Apple. Где AppleAdapter - это класс обслуживания / бизнес-аналитики. Где AppleService является ссылкой Singleton на AppleAdapter, который существует вне текущего метода.

private static volatile AppleAdapter _appleService = null;
private static object _appleServiceLock = new object();
private AppleAdapter AppleService
{
    get
    {
        if (_appleService == null)
        {
            lock (_appleServiceLock)
            {
                if (_appleService == null)
                    _appleService = new AppleAdapter();
            }
        }
        return _appleService;
    }
}

public SomeAppleRelatedMethod(Apple apple)
{
    AppleService.UpdateData(apple);
}

Я не думаю, что то, что вы делаете, не обязательно обязательно, но если SomeDataAdapter действительно представляет какую-то бизнес-службу без сохранения состояния, то лучшим вариантом для этого будет синглтон. Надеюсь, это поможет! Приведенный пример является причудливым способом гарантировать отсутствие конкуренции за _appleService, если он оказался одновременно нулевым и к нему одновременно обратились два или более потоков.

Знаешь что? Если SomeDataAdapter является ADO IDbDataAdapter (которым он почти наверняка является), не обращайте внимания на весь этот ответ!

У меня нет разрешения на добавление комментария к исходному вопросу, но если бы вы могли указать, где этот код существует.

Если этот код представляет пользовательскую реализацию IDbDataAdapter, а UpdateData создает IDbConnection, IDbCommand и подключает все это за кулисами, то нет, я бы не стал считать, что запах кода, потому что теперь мы говорим о потоках и других вещи, от которых нужно избавляться, когда мы закончим, используя их.

Эндрю Хоффман
источник
12
Помогите! Я тону в шаблонах программного обеспечения.
Роберт Харви
4
... или вводится как зависимость. ;)
Роб
12
Неразбериха с синглетонами и двойной проверкой блокировки, чтобы получить что-то, эквивалентное простой функции / процедуре, - намного больший запах кода для меня!
Бен
3

Я думаю, что эта проблема идет еще глубже, чем это. Даже если вы реорганизуете его в статический метод, вы получите код, в котором вы будете вызывать один статический метод повсюду. Что, на мой взгляд, само по себе является запахом кода. Это может указывать на то, что вам не хватает какой-то важной абстракции. Возможно, вы захотите создать код, который будет выполнять некоторую предварительную и последующую обработку, позволяя вам изменять то, что происходит между ними. Эта предварительная и последующая обработка будут содержать общие вызовы, в то время как промежуточное значение будет зависеть от конкретной реализации.

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

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

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

Doval
источник
Это также может означать, что вы хотите, чтобы статический метод возвращал измененную версию (или измененную копию) объекта, который вы ему передаете.
Роберт Харви
2
«если вы застряли в языке без функций первого класса»: нет разницы между объектом, имеющим только один метод, и функцией первого класса (или замыканием первого класса): это просто два разных представления одного и того же вещь.
Джорджио
4
@ Джорджио Теоретически, нет. На практике, если ваш язык не по крайней мере, синтаксический сахар для него, это разница между fn x => x + 1и new Function<Integer, Integer>() { public Integer eval(Integer x) { return x + 1; }};или ограничивая себя несколько жестко закодированных и узко-полезных функций.
Доваль
Почему не может fn x => x + 1быть синтаксического сахара для new Function<Integer, Integer>() { public Integer eval(Integer x) { return x + 1; }}? Хорошо, может быть, вы имеете в виду, если вы застряли в языке без такого синтаксического сахара.
Джорджио
@ Джорджио Вы должны спросить Оракула. (Конечно, с Java 8 это наконец-то меняется). Обратите внимание, что вам понадобится больше синтаксического сахара, чем просто для того, чтобы он был действительно полезным - вы также хотели бы иметь возможность передавать предварительно объявленные методы по имени. Карри также было бы удобно. В какой-то момент вы должны задаться вопросом, стоит ли иметь все эти хаки вместо того, чтобы относиться к функциям как к первым гражданам. Но сейчас я получаю не по теме.
Доваль
0

Если владение государством делает данный класс более утилитарным, функциональным… многократно используемым, то нет. По какой-то причине шаблон проектирования команд приходит на ум; эта причина, конечно, не в желании утопить Роберта Харви.

radarbob
источник
0

Определите «часто». Как часто вы создаете объект? Многое - наверное нет?

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

Однако, если состояние портится, и вам нужно начать делать другие вещи статичными, чтобы сделать первое статичным, тогда вам нужно спросить, какие части должны быть статичными, а какие нет.

Да , это запах кода, просто маленький.

user1775944
источник
0

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

davidk01
источник
0

Запах здесь в том, что это процедурный код, завернутый (минимально) в объекты.

Должна быть причина, по которой вы хотите выполнить эту операцию над таблицей данных, но вместо того, чтобы моделировать эту операцию с помощью других связанных операций, которые имеют некоторую семантику (т.е. имеют общее значение), автор просто развернул новую процедуру внутри новой класс и назвал это.

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

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

КСТАТИ остерегайтесь в любое время, когда вы видите моделируемую программу вместо проблемного пространства . Это признак того, что дизайн может выиграть от большего количества размышлений. Другими словами, следите за тем, чтобы все классы были «xAdaptor», «xHandler» и «xUtility» и обычно привязаны к модели анемичной области . Это означает, что кто-то просто объединяет код для удобства, а не фактически моделирует концепции, которые он хочет реализовать.

обкрадывать
источник