Взаимные эксклюзивные свойства

Вопрос, вероятно, довольно прост, но я хотел бы услышать, какие недостатки у нас будут с нашим кодом.

Итак, у нас была простая реализация и интерфейс для него:

public interface ISettings
{
    bool IsOnline { get; set; }
}

public class Settings : ISettings
{
    public bool IsOnline { get; set; }
}

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

Мой коллега настаивает на реализации дополнительного имущества:

public interface ISettings
{
    bool IsOnline { get; set; }
    bool IsOffline { get; set; }
}

public class Settings : ISettings
{
    public bool IsOnline { get; set; }
    public bool IsOffline
    {
        get { return !IsOnline; }
        set { IsOnline = !value; }
    }
}

Хорошо ли иметь такие дополнительные свойства только для удобного чтения кода?

74 голоса | спросил Kyrylo M 16 +04002013-10-16T11:11:56+04:00312013bEurope/MoscowWed, 16 Oct 2013 11:11:56 +0400 2013, 11:11:56

9 ответов


102

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

Но когда у вас есть два свойства (и не знаю их реализации), вы начинаете задаваться вопросом: есть ли какой-то третий случай? Могут ли оба свойства быть true, или оба false? Связаны ли эти два свойства, или это просто путает имена?

Хорошая документация ответила бы на все эти вопросы, но всегда лучше, если что-то ясно с самого начала и не нужно объяснять.

ответил svick 16 +04002013-10-16T12:22:25+04:00312013bEurope/MoscowWed, 16 Oct 2013 12:22:25 +0400 2013, 12:22:25
131

Простое примечание: перечисление Status (или ConnectionStatus) с ONLINE и OFFLINE) будет более читаемый здесь.

ответил palacsint 16 +04002013-10-16T11:49:55+04:00312013bEurope/MoscowWed, 16 Oct 2013 11:49:55 +0400 2013, 11:49:55
16

Если вы реализуете оба поля, вы в значительной степени должны гарантировать на всю оставшуюся жизнь, что они всегда будут согласованными, то есть IsOnline ==! IsOffline всегда действителен в любой момент времени навсегда.

Вы можете?

Что делать, если кто-то расширяет ваш класс и решает также «расширить» логику.

public class MySettings : Settings
{
    public bool IsOnline { get; set; }
    public bool IsOffline { get; set; }
}

Не является ли MySettings теперь подтипом настроек и может использоваться везде, где это возможно?

Что, если через несколько лет кто-то решит, что им нужно третье государство: «выход в Интернет запускается, но еще не подтвержден», и вместо правильной реинжиниринга код просто решает настроить его так, что IsOffline == IsOnline == false в течение короткого времени?

Что делать, если ваш код используется в многопоточной среде? Если один поток устанавливает IsOnline в какое-то значение, а другой поток одновременно считывает IsOffline? Можете ли вы гарантировать атомарность, следовательно, безопасность потока навсегда?

Я думаю, что если что-то представляет один бит информации, он должен быть одним битом в коде.

ответил wallenborn 16 +04002013-10-16T20:01:16+04:00312013bEurope/MoscowWed, 16 Oct 2013 20:01:16 +0400 2013, 20:01:16
12

Я бы избегал дополнительного свойства. Это совершенно необязательно.

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

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

ответил JᴀʏMᴇᴇ 16 +04002013-10-16T15:54:59+04:00312013bEurope/MoscowWed, 16 Oct 2013 15:54:59 +0400 2013, 15:54:59
6

Если «offline» на самом деле просто «не онлайн» с точки зрения бизнес-логики здесь - скорее всего, - тогда этот метод лишний.

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

Например, допустим, у нас есть метод isAdminUser(), и мы знаем, что «у пользователей admin нет аватаров», поэтому вместо:

if (!isAdminUser()) {
   paintAvatar();
}

... вы делаете:

bool hasAvatar(){
   return !isAdminUser();
}

// ...

if (hasAvatar()) {
   paintAvatar();
}

У вас есть возможность расширить логику hasAvatar() - возможно, аватар стал необязательным, или пользователи-администраторы в каком-то состоянии могут иметь аватар?

Теперь, когда вышеприведенный пример настолько тривиален, вернемся к isOnline /isOffline. Если isOffline только «не в сети», то отлично. Однако, если на самом деле это означает, что вы можете либо иметь «аутентифицированный пользователь», либо «анонимный», никто не будет утверждать, что вы можете легко иметь два метода: isAuthenticated() и isAnonymous(), потому что позже вы можете разбить isAuthenticated() на несколько методов, в то время как isAnonymous() будет продолжать служить своей целью, когда код специально фокусируется на анонимном доступе блоки.

ответил uiron 17 +04002013-10-17T18:49:18+04:00312013bEurope/MoscowThu, 17 Oct 2013 18:49:18 +0400 2013, 18:49:18
5

Да, может быть полезно добавить свойство, подобное этому:

  1. Тестирование положительного результата в состоянии обычно делает его немного легче читать, особенно если условие немного сложнее. Например.

    if (!IsOnline || ForceReset)
    {
        TryReconnect();
    }
    

    против

    if (IsOffline || ForceReset)
    {
        TryReconnect();
    }
    

    В значительной степени зависит от того, кто читает код и как специфицируется спецификация (если она существует). Если спецификация говорит: Попробуйте подключиться, если система отключена или сброс был принудительно , тогда вторая версия легче подключиться к спецификации, чем первая.

  2. Если у вас есть привязка к пользовательскому интерфейсу (например, для WPF), то наличие второго свойства может быть ценным. Обычно такие вещи, как отключение пользовательского элемента управления, выполняются установкой для свойства IsEnabled значения false. Теперь представьте, что вам нужно включить кнопку, если система отключена. Намного проще связать, если доступно свойство IsOffline.

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

ответил ChrisWue 16 +04002013-10-16T12:21:38+04:00312013bEurope/MoscowWed, 16 Oct 2013 12:21:38 +0400 2013, 12:21:38
4

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

Для интерфейса очень важно, чтобы то, что вы определяете, было ясным и недвусмысленным - если вы предоставляете два свойства, потребитель из INTERFACE не может быть уверен, что они являются взаимоисключающими. Даже если вы задокументируете, что они ДОЛЖНЫ реализовать, это может легко игнорировать или иметь ошибку, так что это не так.

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

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

ответил jmoreno 17 +04002013-10-17T09:08:45+04:00312013bEurope/MoscowThu, 17 Oct 2013 09:08:45 +0400 2013, 09:08:45
4

Как @palacsint упоминается Я бы создал свойство Status, но также я будет создавать логические getters только для чтения:

public interface ISettings
{
    Status Status { get; set; }
    bool IsOnline { get; }
    bool IsOffline { get; }
}

public class Settings : ISettings
{
    public Status Status { get; set; }
    public bool IsOnline { get { return Status == Status.Online; } }
    public bool IsOffline { get { return Status == Status.Offline; } }
}

Наличие такого свойства, как Status, делает ваш код более расширяемым. Кроме того, наличие одного статуса с одним сеттером устраняет проблему безопасности потока, которую вы бы имели с двумя доступными свойствами IsOnline IsOffline.

ответил kaptan 18 +04002013-10-18T21:34:23+04:00312013bEurope/MoscowFri, 18 Oct 2013 21:34:23 +0400 2013, 21:34:23
3

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

public static class ISettingsExt
{
    public static bool IsOffline(this ISettings obj)
    {
        return !obj.IsOnline;
    }

    public static void IsOffline(this ISettings obj, bool isOffline)
    {
        obj.IsOnline = !isOffline;
    }
}

Затем любой класс, реализующий интерфейс ISettings, сможет инвертировать логику «IsOnline» без необходимости ее выполнять.

ответил moron4hire 16 +04002013-10-16T19:44:53+04:00312013bEurope/MoscowWed, 16 Oct 2013 19:44:53 +0400 2013, 19:44:53

Похожие вопросы

Популярные теги

security × 330linux × 316macos × 2827 × 268performance × 244command-line × 241sql-server × 235joomla-3.x × 222java × 189c++ × 186windows × 180cisco × 168bash × 158c# × 142gmail × 139arduino-uno × 139javascript × 134ssh × 133seo × 132mysql × 132