Mutually exclusive properties

Question

The question is probably quite simple, but I would like to hear what drawbacks we will have with our code.

So, we had a simple implementation and interface for it:

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

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

The property shows the application state and used in many conditions. The interface in our case is required for implementing settings for different platforms.

My colleague insists on implementing an additional property:

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; }
    }
}

Is it a good practice to have such additional properties just for convenient code-reading?

Answer 1

I think that having both properties is actually somewhat harmful. If there is just single property, then it's clear what it means: settings can be either online or not.

But when you have two properties (and don't know their implementation), then you start to wonder: is there some third case? Can both properties be true, or both false? Are the two properties actually related, or is it just confusing naming?

Good documentation would answer all those questions, but it's always better if something is clear from the start and doesn't have to be explained.

Answer 2

Just a quick note: a Status (or ConnectionStatus) enum with ONLINE and OFFLINE values would be more readable here.

Answer 3

If you implement both fields you pretty much have to guarantee for the rest of your life that they will always be consistent, i.e. that IsOnline==!IsOffline is always valid, at any point in time, forever.

Can you?

What if someone extends your class and decides to "extend" the logic, too.

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

Isn't MySettings now a subtype of Settings and can be used wherever settings can?

What if a few years down the road someone decides that they need a third state: "going online is triggered but not yet confirmed" and instead of properly reengineering the code just decides to tweak it such that IsOffline==IsOnline==false for a short time?

What if your code is used in a multithreaded environment? If one thread sets IsOnline to some value and another thread concurrently reads IsOffline? Can you guarantee atomicity, hence thread-safety forever?

I think if something represents one bit of information it should be one bit in the code.

Answer 4

I would avoid the additional property. It's entirely unnecessary.

The benefits of including it would be to improve readability (ever so slightly) as suggested by ChrisWue's answer. Other than that it poses no advantages that would outweigh the confusion it could cause as suggested by svick (a real consideration to make if it's going to be a headache for future developers).

The whole idea of a Boolean is that it be either true or false (it already caters for both scenarios). You're undermining the purpose of a Boolean merely by creating a negated version of one.

Answer 5

If "offline" is really just a "not online" in terms of business logic here - most probably is - then the method is superfluous.

However, it is not uncommon to create similarly looking methods that from the looks of do not do anything at first glance, but serve as a placeholder to better indicate a test - and be a single point of extension should an underlying logic change later.

For example, let's say we have a method isAdminUser(), and we know what "admin users do not have avatars", so instead of:

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

... you do:

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

// ...

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

You later have the option to extend the logic of hasAvatar() - maybe avatar became optional, or admin users on some condition can have avatar?

Now that above example is so trivial, let's get back to isOnline/isOffline. If isOffline is only "is not online", then fine. However, if what it actually means is that you can either have an "authenticated user" or an "anonymous", nobody will argue that you can easily have two methods isAuthenticated() and isAnonymous(), because you can later break down isAuthenticated() into multiple methods, while isAnonymous() will continue to serve it's purpose where code specifically focuses on anonymous access blocks.

Answer 6

Yes, it can be beneficial to add a property like this:

1. Testing for a positive outcome in a condition usually makes it a little bit easier to read especially if the condition is a bit more complex. E.g.

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

   vs

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

   Highly depends on who reads the code though and how the spec (if it exists) was written. If the spec says: Try to reconnect if the system is offline or a reset was forced then the second version is easier to connect to the spec than the first one.

2. If you have some UI binding (for WPF for example) then having the second property can be valuable. Usually things like disabling a user control is done by setting the IsEnabled property to false. Now imagine you need to have a button enabled if the system is offline. Much easier to bind if the IsOffline property is available.

That being said: It adds additional code which needs to be maintained and unit tested (instead of two states you now have to test four states to make sure it behaves as expected) so don't go blindly around adding a negated version of very boolean property to your models. I'd do it when testing for both the negated and non-negated version is common.

Answer 7

As @palacsint mentioned I would create a Status property, but also I would create read-only boolean 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; } }
}

Having a property like Status makes your code more extendible. Also having one Status with one Setter will eliminate the thread-safety issue you would have with two writeable IsOnline IsOffline properties.

Answer 8

I agree with the other answers to this question, particularly palacsint's, but feel they do not focus on what I consider the most important point: you are talking about an interface.

For an interface, it is crucial that what you define be clear and unambiguous--if you provide two properties, then a consumer of the INTERFACE can not be sure that they are mutually exclusive. Even if you document that they SHOULD an implementation could easily either ignore that or have a bug such that it is not the case.

A particular class might have a justification for having two properties and given that one could be private and implemented as a negatation of the other, one might say no harm no foul.

But an interface is expected to have more than one implementation, and all an implementaion is required to do is implement the required structures, not do so in a particular manner. Introducing this ambiguity does not improve an interface.

Answer 9

You could sort of do it with the MixIn pattern. It will be a method instead of a property, but hopefully that isn't the end of the world for you. Add this static class somewhere in the same namespace as your ISettings interface:

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;
    }
}

Then, any class that implements the ISettings interface will be able to invert the logic of "IsOnline" without personally having to implement it.