0

Is there a design pattern or structure that helps to organize code with variables whose values are related?

For example, let's imagine I have an object with three properties (gonna use some pseudo code):

isVisible: true | false;
activeUser: User | null;
type: 'editable' | 'read-only' | null;

The issue here is that, only some specific states are valid. For example, if isVisible = false, then activeUser and type should be null. However, I also know that type should not be null if isVisible = true.

Same with activeUser and type themselves, if activeUser = null, then type value must be 'editable'.

Is there a way to enforce that kind of relations?

I have tried for example, setting every value as private and then, providing methods that will enforce the necessary values but that doesn't prevent a future developer from setting wrong values from inside the class since the intent and relations between the values are not clearly communicated in the code.

EDIT:

For reference, possible states are:

{
  isVisible: false,
  activeUser: null,
  type: null,
}
{
  isVisible: true,
  activeUser: null,
  type: 'editable',
}
{
  isVisible: true,
  activeUser: User,
  type: 'read-only',
}

How to prevent a developer to code:

{
  isVisible: false,
  activeUser: User,
  type: 'editable',
}
2

The best approach is typically to make the invalid states impossible to express.

For example the isVisible property seem to be redundant, since this is given by the value of user. So remove this property (or make it into a method which checks the if user is null) and you have eliminated that issue.

I cant really follow the rest of the logic (sorry), but if you write down all allowed states, you can probably represent them in a way where illegal combinations are not possible.

| improve this answer | |
  • Thanks! I've edited the question with the list of possible states. I've noticed that everything can be traced down to the activeUser variable being null or not, however, my real use case it's a bit more complicated and you still can't remove all the variables. I'm gonna mark it as an answer anyway because it still answers the question and can help other people. – Lars Dec 2 '19 at 11:40
  • @Lars: If you literally have only 3 possible states, you can make the constructor private and provide 2 static factory methods: CreateInvisibleUser(), CreateUser(type, user). – Filip Milovanović Dec 2 '19 at 16:50
2

The simple approach

For example, if isVisible = false, then activeUser and type should be null. However, I also know that type should not be null if isVisible = true. Same with activeUser and type themselves, if activeUser = null, then type value must be editable.

Firstly, for the simplest approach (too simple, but I'll get into that later) what you're asking for is simple validation logic. This can be done from an object's constructor - but do abstract it into a private method of its own.

public enum MyType { None, Readonly, Editable }

public class Foo
{
    public Foo(User user, bool isVisible, MyType type)
    {
        Validate(user, isVisible, type);

        this.User = user;
        this.IsVisible = isVisible;
        this.Type = type;
    }

    private void Validate(User user, bool isVisible, MyType type)
    {
        if(isVisible == false && user != null)
            throw new Exception("User must be null when not visible");

        if(isVisible == false && type != MyType.None)
            throw new Exception("Type must be None when not visible");

        // and so on...
    }

    public User User { get; set; }
    public bool IsVisible { get; set; }
    public MyType Type { get; set; }
}

This gets you what you want, but it's not a great solution. It requires the consumer to know the exact validation rules in order to get through the constructor, and developers are most likely going to be running into exceptions and having to guess their way through until they get it right. That's not a nice developer experience.

So can we do better? Well, yes.


The better approach

The issue here is that, only some specific states are valid.

The issue here is that you have specific states in mind, but you have not explicitly listed them. This is essential to being able to cover the basics. For example, it is unclear to me why invisible objects must have no user. There's probably a reason for it - but you're not making that information available to me, the consumer of your code, and future developers of your code. That's a problem.

Whenever you're dealing with a bunch of properties but you don't want to allow every possible combination, you're trying to express a concept (these valid states) as a bunch of properties (the value combinations).
The problem here is that even if you manage to implement the correct value combinations, it will not be clear to other developers that you are limiting the code to specific value combinations because of these specific states you want to use (and not others), and what the meaning of these valid states is.

In a professional context, I would either send this back to the analyst/PO for clarification, or I'll approach them myself to work it out. These "states" are clearly an important concept to the domain, so they must be explicitly expressed rather than sneakily foisted on the domain by some undocumented validation logic.

This is where the answers diverge based on what you actually need in your domain. There are different approaches for different situations.

If these states are (each) a collection of hardcoded values, then you can use statics to your advantage:

public class MyState
{
    public string MyString { get; private set; }
    public int MyInt { get; private set; }

    private MyState() { }

    public static MyState StateA => new MyState() { MyString = "Foo", MyInt = 1 };
    public static MyState StateB => new MyState() { MyString = "Bar", MyInt = 2 };
    public static MyState StateC => new MyState() { MyString = "Baz", MyInt = 3 };
}

The static names (StateA etc) should be replaced with descriptive names, but this is a generic example.

Notice the private constructor, it forces consumers to make use of the predefined static properties.

Since you can't just use hardcoded values (the user and type are partially free to choose by the consumer), you need to allow for consumer input. This can be achieved by changing the static properties to static methods:

public class Foo
{
    public User User { get; set; }
    public bool IsVisible { get; set; }
    public MyType Type { get; set; }

    private Foo() { }

    public static Foo Invisible() => new Foo() { User = null, IsVisible = false, Type = MyType.None };
    public static Foo Visible(User u, MyType type) => new Foo() { IsVisible = true, User = u, Type = type };
}

This is an oversimplified example. I haven't implemented your specific rules because they are still pending a better analysis, and there are some variations possible here.

E.g. if you use C#8's non-nullable reference types, you're able to enforce that non-null objects are passed into the method. If not, then you're going to be stuck having to write explicit null checks (which throw an exceptions) or making User a struct (since those are non-nullable by default).

Similarly, if you remove None from the MyType enum, you're able to force developers to pick a not-None value whenever you need it. Then again, enums aren't great validators as you are technically able to input values that the enum doesn't even list, e.g MyType fake = (MyType)12345;
How you want to deal with that very much depends on the contextual expectations of what the type expresses and how you expect to handle it in your domain. Does this enum pose a security backdoor? Can you trust your developers to stick to the listed enum values? What happens if they don't? These considerations matter, a lot.

The point here is that you define the states in order to make it clear to developers and consumers alike that there is a preset list of possible states for the object to be in.


Calculated properties

While I was writing this answer, you listed the allowed states:

{ isVisible: false, activeUser: null,  type: null        }
{ isVisible: true,  activeUser: null,  type: 'editable'  }
{ isVisible: true,  activeUser: User,  type: 'read-only' }

This is actually ripe for optimization, as isVisible can be calculated:

public bool IsVisible => ActiveUser == null && Type == null;

Similarly, I surmise from the given examples that for visible objects, the type must be supplied but the User object remains optional. This can be tackled using specific constructors:

public class Foo
{
    public Foo(string type) { ... }

    public Foo(string type, User u) { ... }

    public string Type { get; }
    public User ActiveUser { get; }
    public bool IsVisible => ActiveUser == null && Type == null;
}

If your language supports optional parameters, it can be condensed into a single constructor:

public class Foo
{
    public Foo(string type, User u = null) { ... }

    public string Type { get; }
    public User ActiveUser { get; }
    public bool IsVisible => ActiveUser == null && Type == null;
}

This is just an educated guess based on the examples you've provided. If it's incorrect, it brings me back to my earlier point: you need to explain the requirements better before you can judge what the appropriate implementation should be.


Sidenote: Development expectations

that doesn't prevent a future developer from setting wrong values from inside the class

I think you're failing to distinguish between consumers of your library, and developers of your library. These are very different roles with very different intentions and working knowledge.
I would expect any developer who makes changes to a given class (not just consuming it) to at least understand the purpose of the class, specifically so they do not make wrong alterations (like e.g. allowing for previously invalid value combinations).

As a general guideline, don't try to pin down future developers this way. I know it comes from good intentions, but what happens when the rules change, or maybe it turns out you were wrong about your implementation?

You're trying to create roadblocks under the assumption that you know the unchangeable rules better than the future developer, but those assumptions are more often wrong than they are right.

This is exactly why OCP exists as a guideline. Things should be closed for modification specifically because future developers having to go back and alter your code can lead to bugs.
Instead, what you're currently trying to do is to expect modification but somehow channel it in ways you approve of; which is a futile (though presumably well intentioned) effort.

| improve this answer | |
  • Thanks a lot for your answer, and yeah, I just realized later that in my search for a simpler example, I oversimplified it too much and everything could just be computed from the activeUser variable. Also, other developers here can be myself, what I always try to do is write code that it's self-explanatory and having rules hidden in variable combinations looks like a bad idea as you pointed out. Thanks again for your time to write such an insightful answer, I wish I could upvote you and I'll remember to come back to do so as soon as I can. – Lars Dec 2 '19 at 13:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.