7

Trying to keep with pragmatic programming principles, I'm trying to decide on how to handle user password changes based on the "Tell, Don't Ask" principle.

I have a user object whose password expires every 30 days. I need to be able to show a password expired/change password view if the password is expired. Asking the object if the password is expired (it's state) and then choosing which view to show seems like a violation of the principle.

What is the best way to handle this situation?

Hupperware
  • 979
  • 1
  • 6
  • 15

5 Answers5

4
login
   model.validate();
   return model.show(self);

passwordExpired()
  return View("ChangePassword")

loginSuccess()
  return View("default")

class User
  show(aController)
      if passwordExpired
          return aContoller.passwordExpired()
     else return aContoller.loginSuccess()

Tell, Don't Ask, no exceptions, and it obeys the law of Demeter

Hupperware
  • 979
  • 1
  • 6
  • 15
3

You could throw a PasswordExpired exception from the user object when the password is authenticated, or whatever function you call first on the user.

GavinCattell
  • 3,863
  • 20
  • 22
  • But that's breaking another principle - don't control the flow using exceptions. Expired password is not an exceptional situation. – NOtherDev Mar 26 '12 at 19:51
  • You shouldn't use exceptions for control flow - they're expensive – David Hoerster Mar 26 '12 at 19:52
  • Good points, thanks. However if you want to stick to the 'Tell, Don't Ask' how could you handle this without exceptions? Return a state from the function? – GavinCattell Mar 26 '12 at 19:55
  • Isn't it exceptional? You wouldn't normally expect the password to be expired in your normal program flow. – DanDan Mar 26 '12 at 19:56
  • It is not. It is something you'll encounter when working with an application, using proper data and proper flow. Exception means that something wrong happens, not just something outside the "happy track". – NOtherDev Mar 26 '12 at 20:00
  • But if the password can expire at any time, how can that be predicted? You don't want to be checking return values every time you ask the server something! – DanDan Mar 26 '12 at 20:05
  • Well, as always, it depends. Maybe we should not interrupt our user's work in the middle of something, but check the password validity on login only? – NOtherDev Mar 26 '12 at 20:12
3

You should consider either having the user object have a Validate() method that provides a boolean (like the membership provider contract does) or consider having a Validate() method return some sort of enumeration that indicates the result of the validation (OK, INVALID_PASSWORD, EXPIRED_PASSWORD, etc.).

There's lots of options - throwing an exception should not be one of them if the password is expired. It's bad form and also a performance hit as the runtime has to unwind the stack.

David Hoerster
  • 28,421
  • 8
  • 67
  • 102
  • I never understood this argument...the stack is pretty good at unwinding itself. It's what it is meant to do. – DanDan Mar 26 '12 at 20:02
  • Do a test. Write a console app that throws an exception and writes the output to the console. Run it in debug mode. Notice how long it takes for your exception to be thrown and displayed. There's a lot going on when you throw an exception - it's expensive. I also understand that debugging is also slower than running in release mode, but there's still a lot going on when you throw an exception. – David Hoerster Mar 26 '12 at 20:05
  • That's fine...I'll handle my exception object when it comes along, I'll be in the best place to deal with the issue because it's full of rich information. I don't really care if it take a few extra nanoseconds to construct. – DanDan Mar 26 '12 at 20:12
  • 1
    Who cares how long it takes in debug mode. You don't use exceptions in a tight loop when performance matters, but if you're worried about the cost of a single exception in a situation like a bad login, you're suffering from pre-mature optimization syndrome, get over it, exceptions are fine until a profiler says they're not. Lisp and Smalltalk call exceptions notifications and conditions. Exception is a subclass of notification, i.e. they are useful for control flow in non exceptional situations. It is not correct to avoid exceptions for performance reasons unless a profiler says so. – Ramon Leon Mar 26 '12 at 20:12
  • 4
    It is not bad form to throw an exception for a password expiration, which is by the way an exceptional condition, i.e. it is not the primary success path. Catch InvalidPassword or ExpiredPassword and render the appropriate view, exceptions work well here, are elegant, and will not cost you in performance. The time it takes to throw a single exception in release mode is simply not worth considering when it'll take orders of magnitude long to hit the db and validate the password. – Ramon Leon Mar 26 '12 at 20:17
  • It will be slower in release mode, too, to say comparing to a null value or checking the return value. The main point is that exceptions should be thrown in exceptional cases, and not used to control the flow of logic. They're generally thrown to handle those situations that are out of your control (a file not being found, a database not available, etc.), not part of logic that should be expected (password expired, end of file reached, etc.). I think the impact will be more noticeable than nanoseconds, regardless of runtime mode. – David Hoerster Mar 26 '12 at 20:18
1

I personally don't like to program arround return values / Enum types. The more return types you have, the more paths you have to test / work with. Also, using exceptions to control flow is imo a bad practice (Unless you really can't find any other option - but there's usually a better one).

An expired password is not really exceptional to me. Its a valid state after all (Or else you'd do something against passwords to expire at all)

I try to keep it simple and either return a bool or something like a Func<T> which can be directly invoked by the caller.

Probably something like that:

public class User
    {
        private DateTime _lastChangeDate;
        public Action Validate()
        {
            if (_lastChangeDate >= DateTime.Now.AddDays(-30))
            {
                return new Action(() => this.Login());
            }
            else
            {
                return new Action(() => this.ChangePassword());
            }
        }
        private void Login()
        {
            Console.WriteLine("Login");
        }
        private void ChangePassword()
        {
            Console.WriteLine("Change Password");
        }
    }

On the caller side:

user.Validate().Invoke();
Alex
  • 7,901
  • 1
  • 41
  • 56
0

One way to solve this is an OO modelling like this:

public class Login {

private String userName;
private String password;
private Date expirationDate;    

public void authenticate(String password) {
    if (this.password.equals(password) {
        redirectoToMainOrEditPage();
    } else {
        redirectToFailPage();
    }
}

private void redirectToMainOrEditPage() {
    Date today = new Date();

    if (today.before(expirationDate)) {
        redirectToMainPage();
    } else {
        redirectToEditPage();
    }
}

private void redirectToMainPage() {
    ...
}

private void redirectToEditPage() {
    ...
}

private void redirectToFailPage() {
    ...
}

public void changePassword(String newPassword) {
    ...
}

public void changeExpirationDate(Date newDate) {
    ...
}
}

This way you don't ask anything to other domain objects, but tell Login to authenticate, as it has everything needed to do this.

user2014029
  • 11
  • 1