Jacob Kur wrote:ok I figured out that the code did not deploy at all due an error .... Duh!!!
Paul Sturrock wrote:Two mappings for the same entity so you can use two different id generation strategies? Why?
Paul Sturrock wrote:What configuration do you want to use? You can either use a generator and delegate to something else to create an id or you can define the id as "assigned" and assign it yourself, you can't do both.
Nathan Pruett wrote:You can implement Spring Security (nee Acegi) as Servlet filters to intercept requests, or annotate models. The view layer library doesn't really impact this decision...
Nathan Pruett wrote:They are really the same project - Spring Security is the new name and Acegi Security was the old name.
David Newton wrote:SalesTax.java
Lines 10-12: Redundant comment.
Lines 15, 17: Constants should be named with all capital letters, with words separated by underscores.
Lines 26-35: Misleading comment: you use numeric values in the comment that are not defined in the method itself: if the constants change and the comments don't you're misleading the code maintainer.
Line 19: There's no reason to have this be an instance variable. (See next.)
Entire class: As written there's no good reason to have this be anything but static methods. If it implemented an interface and there were different ways of calculating the tax I could see it being a class.
Line 64: Misleading comment; uses values not defined locally.
Lines 70-75: I'm not a big fan of redefining parameter values; it's potentially misleading and I believe it should be avoided.
Design: I'm not convinced that an Item should have a sales tax field. Items don't really have sales tax: purchases do (or "line items" do, which is a combination of an item, a quantity, etc.).