На работе возник спор о том как правильно проектировать классы и связывать их между собой.

Есть у нас текст и мы хотим его отправить в одну из N соцсетей (facebook, twitter, evernote, poket ...). Эта задача состоит из подзадачи: подсоединение к соцсети (получение access токена или логина/пароля).

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

Когда я пишу класс для решения задачи я считаю что этот класс должен полностью решать поставленную задачу включая все подзадачи. Т.е. Мой класс решающий проблему шаринга (TextSharer) содержит объект класса ServiceConnector. В итоге когда мы отправляем текст то объект класса TextSharer обработает ситуацию когда пользователь не подключен к сети и делегирует эту проблему внутреннему объекту класса ServiceConnector. Если пользователь уже подключен то шарим сразу.

Мои коллеги сказали что так делать неправильно и надо хранить объекты классов TextSharer и ServiceConnector на одном уровне т/е/ в контроллере (MVC) вида на котором мы хотим шарить текст. Контроллер должен в начале подключиться к сети через объект класса ServiceConnector а затем по результату обратиться к TextSharer и отправить текст. Если запрос на отправку текста у объекта класса TextSharer был вызван без предварительной авторизации то метод объекта TextSharer должен вернуть ошибку.

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

Я понимаю что тема абстрактная и в каждом конкретном случае надо поступать по обстоятельствам но все же. Скажите какой подход вам ближе и почему.

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

Здравствуйте, romca, Вы писали:

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

А если бы нужно было бы написать класс который отправляет на ваш сервер access токены соц сети и с их помощью к примеру выдает кол-во постов. И нужно чтоб он умел работать со всеми соц сетями (у которых этот токен можно получить конечно), в запросе на сервер будут менятся только параметры: имя_соц_сети и токен. Как бы вы написали?

Скажите какой подход вам ближе и почему.

Ну вот есть класс TextSharer. У него есть ответственность за отправку текста в другие соц. сети. Нут так пусть он этим и занимается. Иначе зачем тогда этот класс создавался?

А на счет переиспользования сервисов, пусть лучше временем жизни сервисов занимается DI контейнер, через конфигурацию.

Здравствуйте, maloi_alex, Вы писали:

_>Скажите какой подход вам ближе и почему.

_>Ну вот есть класс TextSharer. У него есть ответственность за отправку текста в другие соц. сети. Нут так пусть он этим и занимается. Иначе зачем тогда этот класс создавался?

_>А на счет переиспользования сервисов, пусть лучше временем жизни сервисов занимается DI контейнер, через конфигурацию.


т.е. не выделять логику подключения к сервисам в ServiceConnector а писать все что в нем прямо в TextSharer? Дело в том что подключение к сервисам нужно в других задачах и пользователь может просто захотеть подключиться к любому сервису в настройках. Т/е/ подключение к сервисам эта подзадача для нескольких разных задач.

Здравствуйте, CatWarrior, Вы писали:

CW>т.е. не выделять логику подключения к сервисам в ServiceConnector а писать все что в нем прямо в TextSharer? Дело в том что подключение к сервисам нужно в других задачах и пользователь может просто захотеть подключиться к любому сервису в настройках. Т/е/ подключение к сервисам эта подзадача для нескольких разных задач.

Просто из описание не очень понятно, какие операции у каждого класса. Если ServiceConnector отвечает за подключение и работу с конкретной соц.сетью, может тогда передавать ServiceConnector в качестве параметра в методы TextSharer. Что-то типа такого:

   serviceConnector.Connect();
   textSharer.ShareText(serviceConnector, text)

Или так:

   serviceConnector.Connect();
   var textSharer = new TextSharer(serviceConnector);
   textSharer.ShareText(text);

Здравствуйте, CatWarrior, Вы писали:

CW>Мои коллеги сказали что так делать неправильно и надо хранить объекты классов TextSharer и ServiceConnector на одном уровне т/е/ в контроллере (MVC) вида на котором мы хотим шарить текст. Контроллер должен в начале подключиться к сети через объект класса ServiceConnector а затем по результату обратиться к TextSharer и отправить текст. Если запрос на отправку текста у объекта класса TextSharer был вызван без предварительной авторизации то метод объекта TextSharer должен вернуть ошибку.

Согласен с вашими коллегами.

CW>Я вот не очень понимаю такой подход, да он гибче, TextSharer ничего не будет знать о ServiceConnector, но тогда если придется использовать TextSharer в других контроллерах, возникнет дублирование кода, т/к/ для выполнения шаринга необходимо быть подключенным к соцсети.

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

Можете посмотреть аналогии в ADO.NET: DbConnection (класс соцсети), DbProviderFactory (у вас нет аналога), DbCommand (TextSharer и прочие операции).
Конечно, ADO.NET решает гораздо более сложные задачи, но идеи можно подсмотреть.

Здравствуйте, Poul_Ko, Вы писали:

P_K>Здравствуйте, CatWarrior, Вы писали:

CW>>Мои коллеги сказали что так делать неправильно и надо хранить объекты классов TextSharer и ServiceConnector на одном уровне т/е/ в контроллере (MVC) вида на котором мы хотим шарить текст. Контроллер должен в начале подключиться к сети через объект класса ServiceConnector а затем по результату обратиться к TextSharer и отправить текст. Если запрос на отправку текста у объекта класса TextSharer был вызван без предварительной авторизации то метод объекта TextSharer должен вернуть ошибку.

P_K>Согласен с вашими коллегами.

CW>>Я вот не очень понимаю такой подход, да он гибче, TextSharer ничего не будет знать о ServiceConnector, но тогда если придется использовать TextSharer в других контроллерах, возникнет дублирование кода, т/к/ для выполнения шаринга необходимо быть подключенным к соцсети.

P_K>В вашем варианте TextSharer берёт на себя слишком много — и шаринг текста, и подключение, и проверку состояния. Эта логика должна быть размещена отдельно.
P_K>Попробуйте написать тесты для вашего TextSharer. Если выяснится, что невозможно ему подсунуть мок соцсети, то ваш подход однозначно не гибче.

P_K>Можете посмотреть аналогии в ADO.NET: DbConnection (класс соцсети), DbProviderFactory (у вас нет аналога), DbCommand (TextSharer и прочие операции).
P_K>Конечно, ADO.NET решает гораздо более сложные задачи, но идеи можно подсмотреть.

Так, хорошо, решили разделить их, но как мне быть если мне нужно шарить из 10 различных мест? Каждый раз в каждом контроллере создавать 2 объекта и реализовывать один и тот-же код связи между TextSharer и ServiceConnector? Тогда если мыслить в выбранном направлении следует написать 3ий класс (паттерн Фасад) который будет связывать логику этих 2ух объектов и давать мне легкий и простой шаринг. Или как?

Здравствуйте, CatWarrior, Вы писали:

CW>но как мне быть если мне нужно шарить из 10 различных мест?

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

   
static class TextSharer
{
    public static void ShareText(string text)
    {
        var connector = ServiceConnector.GetCurrent();
        if (!connector.Connected)
            connector.Connect(); //Исключения в контроллер передам 
        ShareText(connector, text);
    }
    
    public static void ShareText(ServiceConnector connector, string text)
    {
        //Отправка текста
    }
}

Здравствуйте, CatWarrior, Вы писали:

CW>Так, хорошо, решили разделить их, но как мне быть если мне нужно шарить из 10 различных мест? Каждый раз в каждом контроллере создавать 2 объекта и реализовывать один и тот-же код связи между TextSharer и ServiceConnector? Тогда если мыслить в выбранном направлении следует написать 3ий класс (паттерн Фасад) который будет связывать логику этих 2ух объектов и давать мне легкий и простой шаринг. Или как?

Если вам в контроллере не нужно отслеживать подключение/отключение, то как вариант:

// Сервис шаринга текста в какой-то конкретной соцсети
public interface ITextSharingService
{
  void Share(String text);
}

Заводим по одному экземпляру на каждую соцсеть. Реализация может быть одна на все соцсети.
В контроллер передаётся ссылка на реализацию этого интерфейса, вручную или через IoC-конейтнер. При тестировании передаём туда мок.
Вся сложность, связанная с подключением/отключением и т.п., реализована внутри этого сервиса. Там будет ссылка на ServiceConnector. Если алгоритм шаринга сложен или может варьироваться, то он реализуется отдельным классом TextSharer, ссылка на который также будет внутри сервиса. В противном случае алгоритм шаринга реализован в самом сервисе.

Фактически это ваш первоначальный вариант, отличающийся тем, что сервис (ваш TextSharer) передаётся извне, а не создаётся внутри контроллера.
Можно назвать это и фасадом.


Я стараюсь писать именно в таком стиле. Функции контроллера/ViewModel: получение данных от UI, передача их в сервис и обработка результатов. Как там оно реализовано в сервисе контроллер беспокоить не должно. Чтобы контроллер был тестируемым он должен не создавать экземпляр сервиса, а получать его извне.