Strategy Pattern Implementation

Question

I have to generate a code that will send through SMS or Email to implement the One Time Password (OTP) requirement of our client.

I just finished creating the design using strategy pattern, . .

This it the first time I've used strategy pattern in real life projects so I would like to ask if I'm doing it right.

Below is just a sample code that will be used by client.

 //OTPGeneratorStrategy generatorStrategy = new EmailGeneratorStrategy("test@gmail.com"); for sending to Email for example
OTPGeneratorStrategy generatorStrategy = new SmsGeneratorStrategy(993454454545);
OTPGeneratorContextgeneratorContext = new OTPGeneratorContext(generatorStrategy);
context.generate("1-12345-0");

OTPValidatorStrategy validatorStrategy = new SmsValidatorStrategy();
OTPValidatorContext validatorContext = new OTPValidatorContext (validatorStrategy );
try {
validatorContext.validate("1-12345-0","e7241f");
} catch (InvalidVerificationKeyException ivk) {
System.out.println("Verification key is invalid!");
} catch(VerificationKeyExpiredException vke) {
System.out.println("Verification key is already expired, Please regenerate");
}

Updated:

Thank you for the feedback, actually I was not able to update my latest UML design and already separated the generation and validation.

And the reason why I've used Exception rather than enums because the project was built in java 1.4.

Answer 1

First thing that strikes me as a problem is using exceptions for validation result. Don't use exceptions for application logic. Just return an enum value. I would assume there are not many ways validation can end up.

Next thing I would question is using of two strategies. What would happen if you used SMS generator with Email validator? To me, it seems there should be just one strategy with both generate and validate methods. As for "merging the two violates SRP", I would actually say having them separate violates SRP. SRP is about cohesion. And cohesion works both ways. It separates non-cohesive code and joins cohesive code that was split. Ask yourself "If I changed how Email one-time-password policy worked, would I change only generator or validator or would I have to change both?". I'm quite sure you would need to change both, meaning generation and validation are cohesive parts. If you are worried about putting email sending into the strategy implementation, then that should be strategy's concern. OTPContext doesn't need to be bothered by that. It doesn't need to know generation uses some special logic, that is not relevant to validation.

Last nitpick is that I would prefer if strategy was set to OTPContext in a constructor instead of getters/setters. What would happen if you changed the validator strategy after code was generated? You should keep things read-only unless there is explicit requirement for things to change.

Answer 2

As a former, but now somewhat reformed object-oriented programming enthusiast I have to say - be careful with design patterns. This feels like classic "kingdom of nouns" OOP overdesign (check out Steve Yegge's classic blog post on the subject: http://steve-yegge.blogspot.com/2006/03/execution-in-kingdom-of-nouns.html ).

I'd highly recommend trying a simple, procedural implementation of this, in addition to what you're considering - e.g. a simple static method that takes boolean or enum flag for SMS/email, in addition to the other inputs, and follows a simple linear flow of logic, using plain ol' if statements for steps that differ between SMS/email etc.

You may find that the strategy pattern leads to the simplest, most compact, easiest to maintain code in the end, but it definitely pays to try a few design approaches. The more code I've written and read over the years the more I've seen how OOP "best practices" can needlessly overcomplicate things.