Question

I am designing a class that has a state. I wonder if I should expose that state in the interface in view of allowing a decorator to enrich the state transition logic.

Shall my design expose access to the state?

Let's take an example and suppose that I have this interface:

public interface ILoginService
{
    void Login();
    void Logout();
    bool IsLoggedIn { get; } // <-- should that be exposed?
}

One implementation could look like this:

public class LoginService : ILoginService
{
    private bool _loggedIn;
    public IsLoggedIn => _loggedIn;

    public void Login()
    {
        if (!IsLoggedIn) // <-- should that be done?
        { ... }          // Login procedure
    }   
    public void Logout() { ... }   
}

What would be the right approach?

  • Variant A

    Expose the IsLoggedIn property via interface but don't check IsLoggedIn before login procedure within implementation. Reason: If I expose the property, I expect the calling client to handle it correctly. A decorator or client can force the login procedure disregarding the IsLoggedIn property (because of advanced/better knowledge). Disadvantage: Can I expect from implemeters that they must not check the IsLoggedIn property internally?

  • Variant B

    Expose the IsLoggedIn property via interface and check IsLoggedIn before login procedure within implementation. Reason: I can just return in the function and so avoid exceptions by "double login". Disadvantage: A client or decorator (with advanced knowledge) cannot force the implementation to login again, because it always checks the the state itself

  • Variant C

    Don't expose the IsLoggedIn property via interface but check IsLoggedIn before login procedure within implementation. Reason: The client need not care about the state and can trust on the implementation to not "double login". Disadvantage: A decorator cannot decorate the IsLoggedIn property or force the implementation to login again (because the decorator might have some knowledge that the login was rejected / cancelled some time later in background)

Practical problem behind that question: how to enrich the state changes?

In some of our systems the login is discarded if a hardware reset is done. I wanted to handle this via a decorator that is able to detect these resets in order to comply with the SRP.

But for this to work the decorator must be able to force the login, even if the internal state of the implementation says it is already logged in. So I would go for variant A but I am not sure if I can expect to not check the state internally from developers.

Update - My idea of a decorator

public class ResetAwareDecorator : ILoginService
{
    private readonly ILoginService _decoratee;
    private readonly IResetDetector _reset;
    private bool _newResetAfterLastLogin;
    public ResetAwareDecorator(ILoginService decoratee, IResetDetector reset) 
    { ... }

    public bool IsLoggedIn => _decoratee.IsLoggedIn && !_newResetAfterLastLogin; // I would expect the client to test this before calling Login();

    public void Login()
    {
        _decoratee.Login(); // I would expect the decoratee to execute Login() independent of the state _decoratee.IsLoggedIn
        _newResetAfterLastLogin = false;
    }
    public void Logout() { ... }   
}

Are my expectations valid in this scenario?

Was it helpful?

Solution

Are your variants suitable for the job ?

Variant A does not seem to be a good idea: you transfer an internal responsibility to the outside. This breaks the principle of the least knowledge.

Variant B seems perfectly fine: it is legitimate that the external world could query the state of an object if it's more than an implementation detail (e.g. if some UI could need to display the connection status).

Variant C seems ok but doesn't fulfil your needs.

Is it a good idea to use a decorator for this purpose ?

The intent of the decorator pattern is to add a new responsibility dynamically to an object. So it is understandable that you try to add the reset function by this mean.

Unfortunately, you will not only add a new responsibility, but also alter the behavior of the object. So you might break Liskov's substitution principle. Of course, it depends on the guarantee that you offer for ILoginService, but it seems to me that:

  • You do not seem to strengthen the preconditions nor change the invariants
  • You may however weaken the postconditions (depending on how you define them for the interface and your test cases).
  • You will for sure break the history constraint because you allow a change of state that is not foreseen in the super type.

Alternative

In view of the history constraint (LSP), and considering that a login can be interrupted by some event in some implementations of ILoginService, it is clear that the design of the base class should support potential interruption:

  • ILoginService could foresee a method .ForcedLogout. For the login services that don't allow it, you could create a specialization of the method that would trigger an exception.
  • ILoginService could be an observer that could subscribe to some event handler that notify events that may impact the login status. Your base class would have an empty update() method, whereas the specialized one could let its update() access the protected state or protected reset functions.

IMHO, the decorator can do what you need, but is in your case only a work-around of an imperfect design. So I'd advise to go for the alternatives. The second is a little bit more complex, but has the advantage of preventing direct invocation of forced reset for classes that should not allow it.

OTHER TIPS

This is a bit more naive, but what I see it's that your LoginService doesn't really know if it's logged in or not. It just knows how to login and logout and that's fine. Then we just need someone who knows if it's logged in or not. Let's say that someone implements ILoginState and you inject it (i.e. in the constructor maybe?) of the LoginService so when it checks for the IsLoggedIn it calls the ILoginState. Then you can implement the ILoginState as an observer, part of the decorator (which would just add this responsability) or whatever.

Licensed under: CC-BY-SA with attribution
scroll top