1. 1
  1.  

  2. 5

    For user login they suggestion going from:

    User u = UserService.login(username, password);
    

    to

    User u = UserService.getUser();
    

    And claim the latter is better. But what happened to the username and password? Where is the user that is returned from getUser stored? They also make this vague claim that in the first version it’s unclear what you should do with the User when done, “should you log it out?” etc. Ok, but how does the second version solve that problem? How is it clearer that I should or should not log the user out? I don’t see how this is better. It doesn’t even make sense to me.

    Maybe login() returned the User so it could return null when it failed? We are tempted to return error codes from a command if it fails to change state. But it’s better to throw an exception, maintaining the convention that commands return void.

    Why? If you’re language is good and has a result type why wouldn’t I use that instead of exceptions? Comparing exceptions to null is a pretty poor comparison.

    This post is pretty crappy. How do I do anything that requires giving me a proof of my success? For example, logging in often returns a token that you can use in future requests as proof you logged in. What about atomic operations? There might be good reasons to use Command Query Separation (I haven’t seen one) but this article does not make a compelling case.

    1. 2

      I agree. This article fails at a basic level. I don’t see how it really introduces or advocates the topic.

      Just by the name alone (and the “rules” listed late in the post) I would expect you to login a user with one call and retrieve a logged in user with another.

      In the error handling discussion. Why not use the NullObject pattern?

      1. 2

        The point is to separate it into two steps. One that gets the user and one that actually does the login:

        User u = UserService.getUser();
        u.login(username, password);
        

        A clear delineation of usage. If you want the token, you ask for it. No reason to return it here.

        Token t = u.getLoginToken();
        

        No side effect, no confusion, no (possibly) unnecessary value returned.

        1. 2

          That API is not any better as you’ve created a new way to make uninitialized variables, which are fairly universally considered a source of bugs. What happens if I call .getLoginToken, or any other method, on a user that has not had .login called on it or failed to login?

          If I were confronted with that API the first thing I’d do is wrap it in a function called something like doLogin which wraps getting a user, logging it in, and returning it. I never want to see a user that hasn’t logged in.

          If you want to go CQS on this, you probably need a LoginRequest object which has .login and .getUser methods, that way I can only get a User value after it’s logged in. But, again, I’d wrap this up in a doLogin function because the LoginRequest object just exists to live up to CQS and isn’t really providing any value.

      2. 1

        I enjoyed this on-going series of “OO Tricks” - currently 4 short articles on some object oriented programming paradigms. I esp. like #3 and the “Death By Booleans” comment.

        1. 1

          Enforcing no return values from a Command object is kind of ridiculous, there’s no harm in that. If you are doing an operation that changes the state of something, you’ll often want to return the updated thing.

          1. 1

            But that’s the point - you often want it but not always. So just ask for it if you want it.

            1. 2

              Those semantics can be quite confusing in a concurrent program. What if someone changes the object between me doing my thing and asking for the thing?