это способ называть функцию плохой практикой?

10

У меня есть следующий код:

public void moveCameraTo(Location location){
    moveCameraTo(location.getLatitude(), location.getLongitude());
}

public void moveCameraTo(double latitude, double longitude){
    LatLng latLng = new LatLng(latitude, longitude);
    moveCameraTo(latLng);
}

public void moveCameraTo(LatLng latLng){
    GoogleMap googleMap =  getGoogleMap();
    cameraUpdate = CameraUpdateFactory.newLatLngZoom(latLng, INITIAL_MAP_ZOOM_LEVEL);
    googleMap.moveCamera(cameraUpdate);
}

Я думаю, что таким образом я снимаю ответственность за знание того, что есть LatLngв другом классе, например.

И вам не нужно готовить данные перед вызовом функции.

Что вы думаете?

У этого подхода есть имя? Это действительно плохая практика?

Тлалок-ES
источник
2
Я думаю, что вы просто имеете дело с инкапсуляцией, используя встроенную в метод перегрузку методов. Это не хорошо / плохо. Просто инструмент, который может помочь или повредить ваш код.
биткофлогический
5
Если ваша цель скрыть LatLngот клиентов этого Cameraкласса, то, вероятно, вы не хотите moveCameraTo(LatLng)быть public.
биткофлогический
В дополнение к советам, данным в ответах, это хорошее место, чтобы упомянуть YAGNI. Вам это не нужно. Не определяйте методы API, пока не найдется хороший вариант использования, потому что ... он вам не нужен.
Патрик Хьюз
Ничего плохого в moveCameraToLocationи moveCameraTo/ moveCameraToCoords. Определенно не хотел бы передавать Location / lat / long все в тех же именах.
insidesin

Ответы:

9

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

Запах кода здесь будет, если вы продолжите просто расширять цепочку методов, вызывающих методы. Метод получения местоположения вызывает метод двойного получения, который вызывает метод захвата latLng, который, наконец, вызывает то, что знает, как обновить камеру.

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

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

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

Рассматривать:

public void moveCameraTo(Location location){
    moveCameraTo( new LatLng(location) );
}

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

Если Locationэто рефакторимо, но LatLngэто не так, рассмотрите решение этой проблемы, добавив фабрику в Location:

moveCameraTo( location.ToLatLng() );

Это также хорошо избегает примитивной одержимости.

candied_orange
источник
1
Мне кажется, что это не так уж плохо - это просто удобство для потребителя. Если бы это было связано 10 раз или если потребителю не нужны все эти опции, я бы назвал это запахом кода.
MKNZ
1
Но какая альтернатива заключается в том, что вся логика преобразования двойников в LagLng или Location в LagLng в каждой функции?
Tlaloc-ES
@ Tlaloc-ES лучше?
candied_orange
@ Tlaloc-ES «Преобразование» должно произойти только один раз, когда примитивы загружаются в логику домена. Это произойдет, когда вы десериализуете DTO. Затем вся ваша доменная логика проходит через объекты LatLngили Locationобъекты. Вы можете показать отношения между ними с New LatLng(Location)или, Location.toLatLng()если вам нужно перейти от одного к другому.
биткофлогический
1
@ Tlaloc-ES Связанное чтение: Флаг Аргумент Мартина Фаулера. Прочитайте раздел «Запутанная реализация».
Marc.2377
8

В вашем решении нет ничего особенно плохого.

Но я бы предпочел, чтобы эти методы не были такими полезными. И просто усложнить интерфейс любого объекта, от которого они отделены.

Код на void moveCameraTo(double latitude, double longitude)самом деле не упрощает код, так как я не вижу проблем, просто вызывая moveCameraTo(new LatLng(latitude, longitude));его. Этот метод также пахнет примитивной одержимостью.

void moveCameraTo(Location location)Может быть лучше решена путем доказательства Location.ToLatLng()метода и вызова moveCameraTo(location.ToLatLng()).

если бы это был C # и если бы такие методы были действительно необходимы, я бы предпочел их в качестве методов расширения, а не методов экземпляра. Использование методов расширения станет действительно очевидным, если вы попробуете абстрагироваться и протестировать этот экземпляр. Поскольку было бы намного проще просто подделать один метод вместо нескольких перегрузок с помощью простых преобразований.

Я думаю, что таким образом я избавляюсь от ответственности знать, например, что такое LatLng в другом классе.

Я не вижу причин, почему это было бы проблемой. Пока ваш код ссылается на класс, который содержит void moveCameraTo(LatLng latLng), он все еще косвенно зависит от LatLng. Даже если этот класс никогда не создается напрямую.

И вам не нужно готовить данные перед вызовом функции.

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

Размышляя об этом, я чувствую, что то, что я говорю, также поддерживается дизайном API самой .NET. Исторически, многие классы .NET следовали вашему подходу, состоящему в том, чтобы иметь много перегрузок с различными параметрами и простых преобразований внутри. Но это было до того, как появились методы расширения. Более современные классы .NET более легки в своих собственных API, и если есть какие-либо методы с перегрузками параметров, они предоставляются как методы расширения. Более старый пример - NLog ILogger, который имеет десятки перегрузок для записи в журнал. Сравните это с более новым Microsoft.Extensions.Logging.ILogger, который имеет в общей сложности 3 метода (и только 1, если вы считаете сам журнал). Но есть много помощников и различных параметризаций в качестве методов расширения .

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

Euphoric
источник
Не знаю почему, но я чувствую, что болею за этот ответ, несмотря на то, что у меня есть конкурент. Я думаю, что вам просто удалось улучшить положение. Мой единственный отзыв будет помнить, что не все, кто занимается этой проблемой, находятся в .NET. +1
candied_orange
@candied_orange Правильно. Теперь, когда я лучше посмотрю на вопрос OP, это больше похоже на Java, чем на C #.
Euphoric
Я думаю, что на самом деле нет проблем в использовании moveCameraTo(new LatLng(latitude, longitude));в любом месте проекта, но я думаю, что более понятно использовать непосредственно moveCametaTo (latLng), как файл в Java, вы можете передать путь как строку, или как класс Path
Tlaloc-ES
1

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

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

mmathis
источник
1

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

Но не исключается ответственность за знание того, что такое LatLng . Потому что вы инициализируете LatLng latLng = new LatLng(latitude, longitude). Это полная зависимость от LatLng. (Чтобы понять, почему инициализация является проблемой зависимости, вы можете проверить внедрение зависимостей ). Создание перегруженного метода просто помогает клиентам, которым все равно LatLng. Если вы имеете в виду это, это тоже хорошо, но я не думаю, что это подход. Это просто много способов обслуживания клиентов.

Итак, есть два варианта дизайна вашей архитектуры:

  1. Создайте много перегруженных методов и предоставьте их клиенту.
  2. Создайте несколько перегруженных методов, которые нуждаются в параметрах в качестве интерфейса или конкретного класса.

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

Вместо этого используйте интерфейсы (Dependency Injection). Если вы считаете, что это дорого и занимает больше времени, тогда используйте классы и предоставьте методы расширения их картографирования (Вариант 2).

Engineert
источник