Commit 7a882794 by Juho Juopperi

Merge branch 'billrounding' into 'master'

Bill rounding

Make product price scale a constant in Bill. We want to calculate all product prices with 4 decimal precision, just in case. And to make a reference quote for future generations: "This should be enough for everyone". We still want to display the price to users with scale 2, but the scaling should be done at the view level, after all calculations have been made.

This commit does not have any functional changes because all data going through database has scale set already to 4. This just makes the value constant and adds some comments to remind that the scale and rounding mode have real world impact..

By default BigDecimal has a huge scale, which causes BigDecimal to act like a float and these become true:

  * new BigDecimal(555.55) -> 555.549999999999954525264911353588104248046875
  * 55554 == new BigDecimal(555.55).multiply(new BigDecimal(100)).intValue();

See merge request !260
2 parents 2c43b839 095c260d
...@@ -24,6 +24,7 @@ package fi.codecrew.moya.beans; ...@@ -24,6 +24,7 @@ package fi.codecrew.moya.beans;
import java.io.ByteArrayOutputStream; import java.io.ByteArrayOutputStream;
import java.math.BigDecimal; import java.math.BigDecimal;
import java.math.RoundingMode;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Arrays; import java.util.Arrays;
import java.util.Calendar; import java.util.Calendar;
...@@ -173,7 +174,7 @@ public class PlaceBean implements PlaceBeanLocal { ...@@ -173,7 +174,7 @@ public class PlaceBean implements PlaceBeanLocal {
Map<Product, Integer> mockmap = getPlaceProductcount(places); Map<Product, Integer> mockmap = getPlaceProductcount(places);
BigDecimal total = BigDecimal.ZERO; BigDecimal total = Bill.BILL_SCALED_ZERO_PRICE;
Calendar now = Calendar.getInstance(); Calendar now = Calendar.getInstance();
for (Entry<Product, Integer> entry : mockmap.entrySet()) { for (Entry<Product, Integer> entry : mockmap.entrySet()) {
...@@ -182,7 +183,7 @@ public class PlaceBean implements PlaceBeanLocal { ...@@ -182,7 +183,7 @@ public class PlaceBean implements PlaceBeanLocal {
total = total.add(productBean.calculateTotal(entry.getKey(), new BigDecimal(entry.getValue()), now, user)); total = total.add(productBean.calculateTotal(entry.getKey(), new BigDecimal(entry.getValue()), now, user));
} }
} }
return total; return total.setScale(Bill.BILL_PRICE_SCALE, RoundingMode.HALF_UP);
} }
private static Map<Product, Integer> getPlaceProductcount(Collection<Place> places) { private static Map<Product, Integer> getPlaceProductcount(Collection<Place> places) {
...@@ -479,7 +480,6 @@ public class PlaceBean implements PlaceBeanLocal { ...@@ -479,7 +480,6 @@ public class PlaceBean implements PlaceBeanLocal {
return placeFacade.setBuyable(map, like, b); return placeFacade.setBuyable(map, like, b);
} }
/** /**
* Release reservation from user * Release reservation from user
* *
...@@ -724,8 +724,6 @@ public class PlaceBean implements PlaceBeanLocal { ...@@ -724,8 +724,6 @@ public class PlaceBean implements PlaceBeanLocal {
return placeFacade.getMapProducts(map); return placeFacade.getMapProducts(map);
} }
@Override @Override
public List<PlaceSlot> getFreePlaceslots(EventUser user, EventMap map) { public List<PlaceSlot> getFreePlaceslots(EventUser user, EventMap map) {
user = eventUserFacade.reload(user); user = eventUserFacade.reload(user);
......
...@@ -19,6 +19,7 @@ ...@@ -19,6 +19,7 @@
package fi.codecrew.moya.model; package fi.codecrew.moya.model;
import java.math.BigDecimal; import java.math.BigDecimal;
import java.math.RoundingMode;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Calendar; import java.util.Calendar;
import java.util.Date; import java.util.Date;
...@@ -54,6 +55,27 @@ import fi.codecrew.moya.utilities.BillUtils; ...@@ -54,6 +55,27 @@ import fi.codecrew.moya.utilities.BillUtils;
public class Bill extends GenericEntity { public class Bill extends GenericEntity {
/** /**
* <p>
* With how many decimals we want to calculate prices, discounts, etc. If we
* use default scale we will hit rounding problems at some point. By default
* BigDecimal acts as a float and So we get this to be true. Even if we
* calculate prices with scale '4', we most likely want to display the
* prices to users with scale '2'
* <p>
* <p>
* NOTICE! This value exists also in database. If changed, remember to also
* modify the database
* </p>
* <h2>And for future generations, an undying quote</h2> <cite> BigDecimal
* scale 4 should be enough for everybody. - Tuomari<cite>
*/
public static final int BILL_PRICE_SCALE = 4;
public static final BigDecimal BILL_SCALED_ZERO_PRICE = BigDecimal.ZERO.setScale(Bill.BILL_PRICE_SCALE, RoundingMode.HALF_UP);
public static final int VAT_SCALE = 3;
public static final BigDecimal VAT_SCALED_ZERO = BigDecimal.ZERO.setScale(VAT_SCALE, RoundingMode.HALF_UP);
/**
* *
*/ */
private static final long serialVersionUID = -6713643278149221869L; private static final long serialVersionUID = -6713643278149221869L;
...@@ -174,7 +196,7 @@ public class Bill extends GenericEntity { ...@@ -174,7 +196,7 @@ public class Bill extends GenericEntity {
* @return The total sum of the bill ( unitPrice * units * vat ) * @return The total sum of the bill ( unitPrice * units * vat )
*/ */
public BigDecimal totalPrice() { public BigDecimal totalPrice() {
BigDecimal total = BigDecimal.ZERO; BigDecimal total = Bill.BILL_SCALED_ZERO_PRICE;
for (BillLine line : getBillLines()) { for (BillLine line : getBillLines()) {
total = total.add(line.getLinePrice()); total = total.add(line.getLinePrice());
} }
...@@ -191,7 +213,7 @@ public class Bill extends GenericEntity { ...@@ -191,7 +213,7 @@ public class Bill extends GenericEntity {
* @return The total sum of the bill ( unitPrice * units * vat ) * @return The total sum of the bill ( unitPrice * units * vat )
*/ */
public BigDecimal totalVat() { public BigDecimal totalVat() {
BigDecimal total = BigDecimal.ZERO; BigDecimal total = Bill.BILL_SCALED_ZERO_PRICE;
for (BillLine line : getBillLines()) { for (BillLine line : getBillLines()) {
total = total.add(line.getLineVat()); total = total.add(line.getLineVat());
} }
...@@ -204,7 +226,7 @@ public class Bill extends GenericEntity { ...@@ -204,7 +226,7 @@ public class Bill extends GenericEntity {
* @return The total VAT-less sum of the bill ( unitPrice * units ) * @return The total VAT-less sum of the bill ( unitPrice * units )
*/ */
public BigDecimal totalPriceVatless() { public BigDecimal totalPriceVatless() {
BigDecimal total = BigDecimal.ZERO; BigDecimal total = Bill.BILL_SCALED_ZERO_PRICE;
for (BillLine line : getBillLines()) { for (BillLine line : getBillLines()) {
total = total.add(line.getLinePriceVatless()); total = total.add(line.getLinePriceVatless());
} }
...@@ -227,12 +249,12 @@ public class Bill extends GenericEntity { ...@@ -227,12 +249,12 @@ public class Bill extends GenericEntity {
public Bill(LanEvent event, EventUser user, long expireTimeHours) { public Bill(LanEvent event, EventUser user, long expireTimeHours) {
this(event, user, Calendar.getInstance()); this(event, user, Calendar.getInstance());
this.expires.setTimeInMillis((System.currentTimeMillis() + (expireTimeHours*60*60 * 1000 ))); this.expires.setTimeInMillis((System.currentTimeMillis() + (expireTimeHours * 60 * 60 * 1000)));
} }
public Bill(LanEvent event, long expireTimeHours) { public Bill(LanEvent event, long expireTimeHours) {
this(event, Calendar.getInstance()); this(event, Calendar.getInstance());
this.expires.setTimeInMillis((System.currentTimeMillis() + (expireTimeHours*60*60 * 1000 ))); this.expires.setTimeInMillis((System.currentTimeMillis() + (expireTimeHours * 60 * 60 * 1000)));
} }
public Bill() { public Bill() {
...@@ -391,7 +413,7 @@ public class Bill extends GenericEntity { ...@@ -391,7 +413,7 @@ public class Bill extends GenericEntity {
} }
public void setPaidDate(Date paidDate) { public void setPaidDate(Date paidDate) {
if(paidDate != null) if (paidDate != null)
expires = null; expires = null;
this.paidDate = paidDate; this.paidDate = paidDate;
...@@ -462,14 +484,14 @@ public class Bill extends GenericEntity { ...@@ -462,14 +484,14 @@ public class Bill extends GenericEntity {
} }
public boolean isExpired() { public boolean isExpired() {
if(isPaid() || expires == null) if (isPaid() || expires == null)
return false; return false;
return Calendar.getInstance().after(expires); return Calendar.getInstance().after(expires);
} }
public void markExpired() { public void markExpired() {
if(isExpired() || isPaid()) if (isExpired() || isPaid())
return; return;
expires = Calendar.getInstance(); expires = Calendar.getInstance();
...@@ -479,12 +501,12 @@ public class Bill extends GenericEntity { ...@@ -479,12 +501,12 @@ public class Bill extends GenericEntity {
public BigDecimal getTotalQuantity() { public BigDecimal getTotalQuantity() {
BigDecimal total = BigDecimal.ZERO; BigDecimal total = BigDecimal.ZERO;
for(BillLine l : getBillLines()) { for (BillLine l : getBillLines()) {
if(l == null || l.getQuantity() == null) if (l == null || l.getQuantity() == null)
continue; continue;
total = total.add(l.getQuantity()); total = total.add(l.getQuantity());
} }
return total; return total;
} }
...@@ -493,11 +515,11 @@ public class Bill extends GenericEntity { ...@@ -493,11 +515,11 @@ public class Bill extends GenericEntity {
public String getProductSummary() { public String getProductSummary() {
String summary = ""; String summary = "";
for(BillLine l : getBillLines()) { for (BillLine l : getBillLines()) {
if(l == null || l.getQuantity() == null) if (l == null || l.getQuantity() == null)
continue; continue;
if(!summary.isEmpty()) { if (!summary.isEmpty()) {
summary += ", "; summary += ", ";
} }
summary += l.getName(); summary += l.getName();
...@@ -506,10 +528,3 @@ public class Bill extends GenericEntity { ...@@ -506,10 +528,3 @@ public class Bill extends GenericEntity {
return summary; return summary;
} }
} }
...@@ -42,7 +42,6 @@ import javax.persistence.Table; ...@@ -42,7 +42,6 @@ import javax.persistence.Table;
public class BillLine extends GenericEntity { public class BillLine extends GenericEntity {
private static final long serialVersionUID = 2L; private static final long serialVersionUID = 2L;
private static final BigDecimal DEFAULT_VAT = BigDecimal.ZERO;
/** /**
* Which bill this bill line belongs to * Which bill this bill line belongs to
*/ */
...@@ -67,8 +66,8 @@ public class BillLine extends GenericEntity { ...@@ -67,8 +66,8 @@ public class BillLine extends GenericEntity {
* How much one(1) unit of this product costs * How much one(1) unit of this product costs
* *
*/ */
@Column(name = "unit_price", nullable = false, precision = 24, scale = 4) @Column(name = "unit_price", nullable = false, precision = 24, scale = Bill.BILL_PRICE_SCALE)
private BigDecimal unitPrice = BigDecimal.ZERO; private BigDecimal unitPrice = Bill.BILL_SCALED_ZERO_PRICE;
/** /**
* *
...@@ -79,8 +78,8 @@ public class BillLine extends GenericEntity { ...@@ -79,8 +78,8 @@ public class BillLine extends GenericEntity {
/** /**
* How much VAT this product contains ( 0, 0.22 ) etc * How much VAT this product contains ( 0, 0.22 ) etc
*/ */
@Column(name = "vat", nullable = false, precision = 4, scale = 3) @Column(name = "vat", nullable = false, precision = 4, scale = Bill.VAT_SCALE)
private BigDecimal vat = DEFAULT_VAT; private BigDecimal vat = Bill.VAT_SCALED_ZERO;
@JoinColumn(name = "lineProduct_id", referencedColumnName = "id", nullable = true, updatable = false) @JoinColumn(name = "lineProduct_id", referencedColumnName = "id", nullable = true, updatable = false)
@OneToOne @OneToOne
...@@ -111,7 +110,7 @@ public class BillLine extends GenericEntity { ...@@ -111,7 +110,7 @@ public class BillLine extends GenericEntity {
*/ */
public BigDecimal getLinePriceVatless() { public BigDecimal getLinePriceVatless() {
BigDecimal vatMultiplicand = BigDecimal.ONE.add(getVat()); BigDecimal vatMultiplicand = BigDecimal.ONE.add(getVat());
return getLinePrice().divide(vatMultiplicand, 2, RoundingMode.HALF_UP); return getLinePrice().divide(vatMultiplicand, Bill.BILL_PRICE_SCALE, RoundingMode.HALF_UP);
} }
public BillLine() { public BillLine() {
...@@ -157,7 +156,7 @@ public class BillLine extends GenericEntity { ...@@ -157,7 +156,7 @@ public class BillLine extends GenericEntity {
public BillLine(Bill bill2, Product product, Discount disc, BigDecimal count) { public BillLine(Bill bill2, Product product, Discount disc, BigDecimal count) {
super(); super();
this.bill = bill2; this.bill = bill2;
BigDecimal unitPrice = product.getPrice().subtract(product.getPrice().multiply(disc.getPercentage())).negate().setScale(2, RoundingMode.HALF_UP); BigDecimal unitPrice = product.getPrice().subtract(product.getPrice().multiply(disc.getPercentage())).negate().setScale(Bill.BILL_PRICE_SCALE, RoundingMode.HALF_UP);
this.name = disc.getShortdesc(); this.name = disc.getShortdesc();
this.unitName = product.getUnitName(); this.unitName = product.getUnitName();
......
...@@ -19,6 +19,7 @@ ...@@ -19,6 +19,7 @@
package fi.codecrew.moya.model; package fi.codecrew.moya.model;
import java.math.BigDecimal; import java.math.BigDecimal;
import java.math.RoundingMode;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Calendar; import java.util.Calendar;
import java.util.Date; import java.util.Date;
...@@ -45,13 +46,15 @@ public class Discount extends GenericEntity { ...@@ -45,13 +46,15 @@ public class Discount extends GenericEntity {
private static final long serialVersionUID = 1L; private static final long serialVersionUID = 1L;
private static final String EVENT_ID = "event_id"; private static final String EVENT_ID = "event_id";
private static final BigDecimal SCALE4_ZERO = BigDecimal.ZERO.setScale(4, RoundingMode.HALF_UP);
private static final BigDecimal SCALE6_ZERO = BigDecimal.ZERO.setScale(6, RoundingMode.HALF_UP);
@ManyToOne() @ManyToOne()
@JoinColumn(name = EVENT_ID, nullable = false) @JoinColumn(name = EVENT_ID, nullable = false)
private LanEvent event; private LanEvent event;
@Column(name = "percentage", nullable = false, precision = 9, scale = 6) @Column(name = "percentage", nullable = false, precision = 9, scale = 6)
private BigDecimal percentage = BigDecimal.ZERO; private BigDecimal percentage = SCALE6_ZERO;
@Column(name = "code") @Column(name = "code")
private String code; private String code;
...@@ -71,22 +74,22 @@ public class Discount extends GenericEntity { ...@@ -71,22 +74,22 @@ public class Discount extends GenericEntity {
private String shortdesc; private String shortdesc;
@Column(name = "amount_min", nullable = false, precision = 24, scale = 4) @Column(name = "amount_min", nullable = false, precision = 24, scale = 4)
private BigDecimal amountMin = BigDecimal.ZERO; private BigDecimal amountMin = SCALE4_ZERO;
@Column(name = "amount_max", nullable = false, precision = 24, scale = 4) @Column(name = "amount_max", nullable = false, precision = 24, scale = 4)
private BigDecimal amountMax = BigDecimal.ZERO; private BigDecimal amountMax = SCALE4_ZERO;
@Column(name = "active", nullable = false) @Column(name = "active", nullable = false)
private boolean active = false; private boolean active = false;
@Column(name = "max_num", nullable = false, precision = 24, scale = 4) @Column(name = "max_num", nullable = false, precision = 24, scale = 4)
private BigDecimal maxNum = BigDecimal.ZERO; private BigDecimal maxNum = SCALE4_ZERO;
@Column(name = "per_user", nullable = false, precision = 24, scale = 4) @Column(name = "per_user", nullable = false, precision = 24, scale = 4)
private BigDecimal perUser = BigDecimal.ZERO; private BigDecimal perUser = SCALE4_ZERO;
@Column(name = "total_count", nullable = false, precision = 24, scale = 4) @Column(name = "total_count", nullable = false, precision = 24, scale = 4)
private BigDecimal totalCount = BigDecimal.ZERO; private BigDecimal totalCount = SCALE4_ZERO;
@OneToMany(cascade = CascadeType.ALL, mappedBy = "discount") @OneToMany(cascade = CascadeType.ALL, mappedBy = "discount")
private List<DiscountInstance> discountInstances; private List<DiscountInstance> discountInstances;
......
...@@ -22,8 +22,8 @@ ...@@ -22,8 +22,8 @@
*/ */
package fi.codecrew.moya.model; package fi.codecrew.moya.model;
import java.math.BigDecimal; import java.math.BigDecimal;
import java.math.RoundingMode;
import java.util.HashSet; import java.util.HashSet;
import java.util.List; import java.util.List;
import java.util.Set; import java.util.Set;
...@@ -58,11 +58,11 @@ public class Product extends GenericEntity { ...@@ -58,11 +58,11 @@ public class Product extends GenericEntity {
@Lob @Lob
private String description; private String description;
@Column(name = "price", nullable = false, precision = 24, scale = 4) @Column(name = "price", nullable = false, precision = 24, scale = Bill.BILL_PRICE_SCALE)
private BigDecimal price = BigDecimal.ZERO; private BigDecimal price = Bill.BILL_SCALED_ZERO_PRICE;
@Column(name = "buy_in_price", nullable = true, precision = 24, scale = 4) @Column(name = "buy_in_price", nullable = true, precision = 24, scale = Bill.BILL_PRICE_SCALE)
private BigDecimal buyInPrice = BigDecimal.ZERO; private BigDecimal buyInPrice = Bill.BILL_SCALED_ZERO_PRICE;
@Column(name = "sort", nullable = false) @Column(name = "sort", nullable = false)
private int sort; private int sort;
...@@ -70,11 +70,9 @@ public class Product extends GenericEntity { ...@@ -70,11 +70,9 @@ public class Product extends GenericEntity {
@Column(name = "barcode") @Column(name = "barcode")
private String barcode; private String barcode;
@OneToMany(cascade = CascadeType.ALL, mappedBy = "product") @OneToMany(cascade = CascadeType.ALL, mappedBy = "product")
private List<LicenseTarget> licenseTargets; private List<LicenseTarget> licenseTargets;
@Enumerated(EnumType.STRING) @Enumerated(EnumType.STRING)
@PrivateOwned @PrivateOwned
@ElementCollection() @ElementCollection()
...@@ -110,8 +108,8 @@ public class Product extends GenericEntity { ...@@ -110,8 +108,8 @@ public class Product extends GenericEntity {
}) })
private List<Discount> discounts; private List<Discount> discounts;
@Column(name = "vat", nullable = false, precision = 4, scale = 3) @Column(name = "vat", nullable = false, precision = 4, scale = Bill.VAT_SCALE)
private BigDecimal vat = BigDecimal.ZERO; private BigDecimal vat = Bill.VAT_SCALED_ZERO;
private String unitName = ""; private String unitName = "";
@ManyToMany() @ManyToMany()
...@@ -189,6 +187,7 @@ public class Product extends GenericEntity { ...@@ -189,6 +187,7 @@ public class Product extends GenericEntity {
/** /**
* Get product price, includes vat * Get product price, includes vat
*
* @return * @return
*/ */
public BigDecimal getPrice() { public BigDecimal getPrice() {
...@@ -197,6 +196,7 @@ public class Product extends GenericEntity { ...@@ -197,6 +196,7 @@ public class Product extends GenericEntity {
/** /**
* Set price, including vat * Set price, including vat
*
* @param price * @param price
*/ */
public void setPrice(BigDecimal price) { public void setPrice(BigDecimal price) {
...@@ -253,6 +253,7 @@ public class Product extends GenericEntity { ...@@ -253,6 +253,7 @@ public class Product extends GenericEntity {
/** /**
* Set product vat-%, value between 0 and 1 * Set product vat-%, value between 0 and 1
*
* @param vat * @param vat
*/ */
public void setVat(BigDecimal vat) { public void setVat(BigDecimal vat) {
...@@ -261,6 +262,7 @@ public class Product extends GenericEntity { ...@@ -261,6 +262,7 @@ public class Product extends GenericEntity {
/** /**
* Get product vat-%, value between 0 and 1 * Get product vat-%, value between 0 and 1
*
* @return * @return
*/ */
public BigDecimal getVat() { public BigDecimal getVat() {
...@@ -312,10 +314,10 @@ public class Product extends GenericEntity { ...@@ -312,10 +314,10 @@ public class Product extends GenericEntity {
Set<ProductFlag> flags = getProductFlags(); Set<ProductFlag> flags = getProductFlags();
if(flags == null) if (flags == null)
flags = new HashSet<ProductFlag>(); flags = new HashSet<ProductFlag>();
if(!flags.contains(flag)) { if (!flags.contains(flag)) {
flags.add(flag); flags.add(flag);
setProductFlags(flags); setProductFlags(flags);
} }
...@@ -361,7 +363,6 @@ public class Product extends GenericEntity { ...@@ -361,7 +363,6 @@ public class Product extends GenericEntity {
this.licenseTargets = licenseTargets; this.licenseTargets = licenseTargets;
} }
public boolean isUsershopAutoproduct() { public boolean isUsershopAutoproduct() {
return getProductFlags().contains(ProductFlag.USERSHOP_AUTOPRODUCT); return getProductFlags().contains(ProductFlag.USERSHOP_AUTOPRODUCT);
} }
......
Markdown is supported
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!