views:

78

answers:

3

I'm implementing a system where a customer can use vouchers to gain discounts for a purchase. If a voucher can be used for a certain purchase depends on several circumstances.

For example:

  • Proper voucher code - is the code correct?
  • Validity Range - is the voucher still valid?
  • Can the voucher be used with the type of purchase?
  • Combination permitted - can the voucher be combined with other vouchers?
  • many more ...

There are also a few more complex restrictions that need to be checked. If one or more restrictions are not satisfied the customer cant use the voucher and I'd like to inform him/her about the failure with an explanation of "WHY" he cant use this voucher for example:

"You can't use this voucher because it's out of date."

My question now is: How would you implement the checks?
Implement each restriction in a class of its own, chain them and throw exceptions? (Problem here, possible several identical database queries would be executed) Implement all restrictions in one single method? (really, why?) In general, how do you implement a mechanism where you have to inform a client about the details of a failure if complex restriction are applied to an action?

Thanks,

+2  A: 

Personally I would implement the checks in just one method. The voucher goes in, an error message goes out (if any).

This will keep all the code hidden behind that method and make maintenance easy. One place to check if something goes wrong, one place to modify when adding new things etc.

If we are talking a large number of tests, than change the method to a class (but once again responsibility is in one place, without complexity being spread out all over the place).

Just my two cents.

dpb
A: 

I would abstract the intent so that it doesn't really matter how it's implemented, or how the source code is organised. At a basic level what you have is a collection of checkers generating a collection of failure explanations.

The psuedo-code would basically be:

let reasons be a new, empty collection of failure reasons 
let checkers be the list of relevant checkers

for each checker in checkers
    if checker passes, continue
    if checker fails, add explanation to reasons

if number of reasons is zero, 
    voucher is valid, success
if number of reasons > zero, 
    the voucher is invalid, format each element in reasons for display to the user

With this logic in place, it doesn't matter how the checkers are organised, as long as they can be obtained by this part of the code in a list. You could have a single method with many checks, possibly adding many reasons. You could have many classes, with one instance of each in the list of checkers. Crucially, the actual checkers are decoupled from this logic, and different checkers can be used at any time (think of business rules changing, or different rules in different regions).

Depending on your language, this would, at a minimum, involve abstracting the type used for checkers.

Start with that. If you find identical database queries, start to consider a cache for the run of the collection of checkers.

As far as how the checkers are organised on a source level... it doesn't matter. Only the abstraction does. The details are fairly flexible once you provide a level of abstraction you can hide behind.

Pros:

  • The bit that performs the checking doesn't care about the actual, concrete checks (which will probably change based on business rules).
  • The checkers can be organised however you would like: defined together in one source file, or spread out based on some other scheme. It also allows the freedom to separate out a single checker for testing.
  • The collection, and the pretty formatting, only needs to be implemented once, regardless of the type or number of checks.

Cons:

  • It takes time upfront to do a decent job of designing the abstractions. This will probably pay off later.
  • There's a layer of indirection which may make it more difficult to understand, and to track down the actual checks which are being made. Flexibility is to the detriment of understandability. These are things you will have to consider for your scenario. In my opinion, the rules for vouchers seems to be something which is likely to change over time, and the abstractions here are not difficult to understand, so I'd say it's worth it.

I spent time writing an example in Java, but it really didn't add much. If you try and think in terms of the abstraction, the actual mechanism is less important.

Grundlefleck
A: 

I recently dealt with a similar issue checking orders placed using a one-step checkout page. In the end, we found it much more useful to generate an array of all relevant "problems" (we wrapped these in a simple class with a code, display message, etc) rather than a string describing the first problem encountered.

This works with a single checker method that returns an array of relevant problems. The main checker method calls more specific checker methods to build the array. It may choose to skip some checks if others have already failed.

anyaelena