0

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?

  • 1
    I'm less concerned about fulfilling the Single Responsibility Principle than I am about satisfying your objectives. Does your approach adequately satisfy your objectives without the decorator? – Robert Harvey Sep 14 '18 at 20:04
  • When you say "force the login" does that mean you want the system to log out and then log back in? – Robert Harvey Sep 14 '18 at 20:08
  • Thanks for the hint, I edited the spelling. With "force the login" I mean to do the login procedure again, not logging out before. It should just behave if it was logged out. In normal circumstances the LoginService behaves like it should and no decorator is required. But in some of our systems we have to take resets into consideration. Is decorator the wrong approach and I should instead implement an additional ILoginService for these special circumstances? – Creepin Sep 14 '18 at 20:35
  • Can an external reason change the state from logged in to not logged in / disconnected? If so, the state query should be exposed so the client can initiate a reconnect. – Kristian H Sep 14 '18 at 23:44
  • @KristianH yes, an external reason can change that state, but this is just a special requirement of some systems, or better to say an additional requirement to the normal behaviour – Creepin Sep 15 '18 at 6:50
0

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.

  • If variant C would expose IsLoggedIn as getter, it would be variant B effectively, because IsLoggedIn is readonly in all variants. Do you really think it would break the LSP? I edited my post with an idea of a decorator implementation. In my opinion the decorator cares for fulfilling the LSP in systems where resets alter the state, and the original implementation fulfills the LSP in systems where resets doesn't matter. Regarding your first alternative: Wouldn't that break the LSP by throwing an exception? Your second variant would indeed be a possibility, I have to think about it. – Creepin Sep 15 '18 at 6:47
  • 1
    @Creepin Good catch. I think I have misinterpreted your explanation of variant B. So I've edited my answer and elaborated on the problems with LSP. Hope it's clearer now :-) – Christophe Sep 15 '18 at 9:42
  • First, thanks for your edits, sounds much better. I am not sure if the decorator would violate the history constraint. The decorator does not expose any further possibility to change the state in comparison to what the super type (interface) already exposes. .... – Creepin Sep 15 '18 at 15:37
  • But if we assume it would violate the history constraint, then why doesn't your second variant violate it? It also listens internally to an event that notifies on login status impacts. This is similar to what my decorator example does. So in both cases the state is changed internally. I thought of adding an event LoginStateChanged to the interface to give clients the chance to register on internal state changes. – Creepin Sep 15 '18 at 15:38
  • @Creepin my point is that the potential change of status due to external situation has to be designed into the interface from the start and not via decorator. Because it's the only way not to violate history constraint. My two alternatives are only variants of how to change the interface. By making clear that outside events could change the status (especially 2nd option) there's no history constraint anymore – Christophe Sep 15 '18 at 16:15
1

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.

  • 1
    I also thought about that. It tracks down to Christophes second idea I think. Being an observer to some observant. But ILoginState must also be notified by ILoginService about state changes. While ILoginState could be implemented specifically for the system (listening to logins and to resets or only to logins) I feel a bit bad about reacting to an implementation detail in a (base) class. What if I got to know this behavior 2 weeks later and designed and implemented the ILoginService under normal circumstances? – Creepin Sep 16 '18 at 7:49
  • 1
    @Creepin Yep exactly +1. The point is that the ILoginService just handle the procedure but doesn't know the state, so let someone else to manage it (this relates to SRP, right?). That's if preventing 2× login is really needed. This is only a way to do that. – FranMowinckel Sep 16 '18 at 11:49

Your Answer

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

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