ShoppingCart is very broken when dealing with Uom

[Adam Heath-2]

Omg.  I'm floored.

ShoppingCartItem.getSize() does not deal with uom *at all*.  Then,
that method is so very broken; it has no idea how to do volume at all!

length, width, height is used to calculate the volume.  And the
algorithm it uses is...

wait for it...

(length+length)+(width+width)+height

Surface area of a cube: 2a*b + 2b*c + 2a*c
Volume of a cube: a*b*c


getSize() is none of these.

ShoppingCart.getShippableWeight(int) is also very broken.  It doesn't
deal with uom at all.  What if I order a case of something, that is
measured in pounds, then only a singleton item, that is measured in
ounces?  getShippableWeight just sums up the weights of all the cart
items.

I've looked in the history, and these issues have been unchanged since
July, 2006, when ofbiz was imported to apache.

This would also have breakage for any kind of automatic assignment
into shipping containers when planning a shipment.

[Scott Gray-2]

On 16/03/2010, at 6:27 PM, Adam Heath wrote:

> Omg.  I'm floored.
>
> ShoppingCartItem.getSize() does not deal with uom *at all*.  Then,
> that method is so very broken; it has no idea how to do volume at all!
>
> length, width, height is used to calculate the volume.  And the
> algorithm it uses is...
>
> wait for it...
>
> (length+length)+(width+width)+height
>
> Surface area of a cube: 2a*b + 2b*c + 2a*c
> Volume of a cube: a*b*c
>
>
> getSize() is none of these.

Right or wrong, it looks like there was a reason: http://svn.ofbiz.org/viewcvs?rev=3807&view=rev

[Adam Heath-2]

Scott Gray wrote:

> On 16/03/2010, at 6:27 PM, Adam Heath wrote:
>
>> Omg.  I'm floored.
>>
>> ShoppingCartItem.getSize() does not deal with uom *at all*.  Then,
>> that method is so very broken; it has no idea how to do volume at all!
>>
>> length, width, height is used to calculate the volume.  And the
>> algorithm it uses is...
>>
>> wait for it...
>>
>> (length+length)+(width+width)+height
>>
>> Surface area of a cube: 2a*b + 2b*c + 2a*c
>> Volume of a cube: a*b*c
>>
>>
>> getSize() is none of these.
>
> Right or wrong, it looks like there was a reason: http://svn.ofbiz.org/viewcvs?rev=3807&view=rev

Wrong.  If ups needs something special, then ups should do it.

getSize() is too generic of a name.

Fixing this could cause all sorts of funkiness everywhere.  But it
must be done, because what is happening currently is not right in the
slightest.

[Ean Schuessler]

----- "Adam Heath" wrote:
> Wrong. If ups needs something special, then ups should do it.
> getSize() is too generic of a name.
> Fixing this could cause all sorts of funkiness everywhere. But it
> must be done, because what is happening currently is not right in the
> slightest.

Agreed. It should probably return dimensions and let the client code do what it will.

I don't even know if the UPS thing applies anymore (if it ever did): http://www.ups.com/content/us/en/resources/prepare/dim_weight.html 

--
Ean Schuessler, CTO Brainfood.com
[hidden email] - http://www.brainfood.com - 214-720-0700 x 315