mmcconnell1618/MerchantTribe — pull request #3 by NightOwl888 (2012-04-06 18:37:42+00:00)
Title: Added global setting for MidpointRoundingMode

Ok, I implemented a global setting for MidpointRoundingMode so it can be set by the end user. I ended up putting the setting on the Geo-Location page for lack of a better fit (since rounding applies to Orders, Discounts, Shipping, and Tax).

There were 2 issues:

1. The "global" settings could not be accessed in the shipping modules or most other DLLs external to MerchantTribe.Commerce because the DLL reference goes the wrong way.
2. There was no way to access the instance of MTApp from many of the classes where rounding is used.

Issue# 1:

Hard-coding the rounding mode was the solution. Fortunately, the documentation from the shippers and a document on how to calculate sales are very clear on which rounding method to use.

UPS

"For each dimension, measure at the longest point, rounding each measurement to the nearest whole number (for example, 1.00 to 1.49 will be considered 1, and 1.50 to 1.99 will be considered 2)."

Source: http://www.ups.com/content/us/en/resources/ship/packaging/dim_weight.html

FedEx

"Length + Girth = L + (2 x W) + (2 x H) If this total
dimension includes a fraction, a fraction of one-half
or greater will be rounded up; less than one-half
will be rounded down to the next whole number."

Source: http://images.fedex.com/ca_english/shippingguide/preparepackage/packagingbrochure.pdf

USPS

"7.1.3 Rounding Numerical Values
For these standards:

a. Round off requires increasing by 1 the last digit to be kept if the digit to its right, which is not to be kept, is 5 or greater. If that digit is 4 or less, the last digit kept is unchanged (e.g., 3.376 rounded off to two decimal places is 3.38, 3.374 is 3.37).

b. Round up requires increasing by 1 the last digit to be kept if there are any digits to its right, regardless of significance (e.g., rounding up either 3.3701 or 3.379 to two decimal places yields 3.38).

c. Round down requires eliminating any digits to the right of the last number to be kept (e.g., rounding down either 3.371 or 3.379 to two decimal places yields 3.37)."

NOTE: Although the AwayFromZero option is not used in every form of rounding here (just the first one), suffice to say they do not use banker's rounding for any of their calculations. For the other rounding types, Microsoft has this document on how they can be implemented: http://support.microsoft.com/?kbid=196652. I do not plan on fixing this as the USPS provider is not one I will be using.

Source: http://pe.usps.com/text/dmm300/604.htm#xd_604_7_1_3

Sales Tax

"If five or greater, round the sales tax dollar amount up to the next penny and remove the extra digits."

Source: How to Round Sales Tax | eHow.com http://www.ehow.com/how_6524918_round-sales-tax.html#ixzz1rHdk58FF

Issue #2

There were basically 2 options to fix this:

1. Add constructors and method parameters where appropriate to pass an instance of MTApp.
2. Make a static property to access the instance of MTApp associated with the current request.

I opted for the latter because it means fewer changes to the code of the project and also directly relates to my next project (making the product variants scale much better).

If you disagree with this decision I could change it to use the first solution. The first solution allows the creation of more than one MTApp object and the second does not, so using the second solution obviously limits access only to the global store's settings. However, the second option does make for a simpler API to use because you won't always need to figure out how to get an instance of object A before creating an instance of object B.

Note that I used a trick that the CSLA.NET framework uses in order to ensure it will work whether it is in a web environment or a Windows environment. In a Windows environment (when testing), it attaches the MTApp to the current thread instead of using HttpContext.Current.Context.Items since the property obviously won't be available.

Note also that the solution is a compromise. Ideally, the global MTApp object would be a readonly property that can only be set using a friend method that the web site has no access to (similar to how HttpContext.Current is set). However, I made the property read-write in order to make a smaller impact on the rest of the application code.

If you decide to use the global static property, one more decision needs to be made - what to do if it is not set.

Once again there are 2 obvious options:

1. Throw an exception with a message indicating the MTApp must be set
2. Use a singleton pattern to create the instance automatically

Personally, I like the first option because it will ensure the application will fail at runtime if the global context is not correct. If the singleton pattern is used, setting the current store could easily be overlooked when creating test methods and the code would simply use default values that may or may not affect the result of the test.

Note that I put both options in the code and ended up commenting the error message, so if you agree with me that the app should throw an exception in this case you can easily put it in by removing the commented code and commenting the 2 lines above in the get accessor of MerchantTribeApplication.Current.

Lastly, there were the tests. I made each test class globally create an instance of MerchantTribeApplication and set it up before tests are run. Note that for each test the global instance will be reset to prevent any state from transferring from one test to another. Using the singleton pattern will eliminate the need for these extra methods by ensuring the instance always exists, however this could easily lead to the developer who creates new tests from forgetting to set up this global instance properly.

One last thing. If you decide to reject this pull request and accept my first one, I wouldn't blame you. Rounding is something that should be carefully considered on a case-by-case basis and not applied globally. However, as banker's rounding is useful in some situations, in practice it is rarely used. Although "officially" Microsoft states that they use banker's rounding for the default because it is the IEEE standard, it is much more likely the default because the AwayFromZero rounding method wasn't put into the framework until version 2.0. Changing it at that point would have broken thousands of applications.