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 checkIsLoggedIn
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 theIsLoggedIn
property (because of advanced/better knowledge). Disadvantage: Can I expect from implemeters that they must not check theIsLoggedIn
property internally?Variant B
Expose the
IsLoggedIn
property via interface and checkIsLoggedIn
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 itselfVariant C
Don't expose the
IsLoggedIn
property via interface but checkIsLoggedIn
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?
-
1I'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– Robert Harvey09/14/2018 20:04:58Commented Sep 14, 2018 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– Robert Harvey09/14/2018 20:08:17Commented Sep 14, 2018 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– Creepin09/14/2018 20:35:55Commented Sep 14, 2018 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– Kristian H09/14/2018 23:44:16Commented Sep 14, 2018 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 behaviourCreepin– Creepin09/15/2018 06:50:43Commented Sep 15, 2018 at 6:50
2 Answers 2
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 emptyupdate()
method, whereas the specialized one could let itsupdate()
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, becauseIsLoggedIn
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– Creepin09/15/2018 06:47:50Commented Sep 15, 2018 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– Christophe09/15/2018 09:42:01Commented Sep 15, 2018 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– Creepin09/15/2018 15:37:53Commented Sep 15, 2018 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– Creepin09/15/2018 15:38:05Commented Sep 15, 2018 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 anymoreChristophe– Christophe09/15/2018 16:15:30Commented Sep 15, 2018 at 16:15
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.
-
1I 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– Creepin09/16/2018 07:49:27Commented Sep 16, 2018 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– FranMowinckel09/16/2018 11:49:17Commented Sep 16, 2018 at 11:49
Explore related questions
See similar questions with these tags.