Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Order of values appears to be important #14

Open
neodisco opened this issue Oct 11, 2019 · 7 comments
Open

Order of values appears to be important #14

neodisco opened this issue Oct 11, 2019 · 7 comments
Assignees
Labels
Milestone

Comments

@neodisco
Copy link

neodisco commented Oct 11, 2019

Using your example1.php and replacing with...

['length' => 1, 'width' => 1, 'height' => 100] results in "Container 100x1x100(cm) 10000(cm3)"

Whereas

['length' => 100, 'width' => 1, 'height' => 1] results in "Container 100x1x1(cm) 100(cm3)"

Is this a bug and/or do the 3 values need to be sorted desc before adding to length, width and height respectively?

@neodisco neodisco changed the title Order of values appears to be important (not rotating in 3d?) Order of values appears to be important Oct 11, 2019
@mdeboer
Copy link
Member

mdeboer commented Oct 11, 2019

That is weird, the first example should result in a container of 1x1x100 not 100x1x100. Are you sure about the result? Can you also try with the following and let me know what result you get?

[1,1,100] (should result in 1x1x100)

As for the array, as long as you use an associative array (like in your example), the order of the elements does not matter. The order only matters when using an indexed array (like in my example above) in which the order is LxWxH.

@neodisco
Copy link
Author

I confirm.

// Define our boxes
$boxes = array(
	array(
		'length' => 1,
		'width' => 1,
		'height' => 100
	),
);

// Initialize LAFFPack
$lp = new \Cloudstek\PhpLaff\Packer();

// Start packing our nice boxes
$lp->pack($boxes);

creates output of:

Packed boxes
Packed boxes: 1
Total volume: 100(cm3)
Wasted space: 9900(cm3) / 99%

Remaining boxes
Remaining boxes: 0
Total volume: 0(cm3)

Result
Container 100x1x100(cm) 10000(cm3)
Level 0 1x100x100(cm) 100(cm2)
Box 0 1x1x100(cm) 1(cm2)

@mdeboer
Copy link
Member

mdeboer commented Oct 14, 2019

Aight, that is defo a bug. It's time for a rewrite anyway, it is ancient 🤣 Just haven't got the time at the moment, PRs are welcome though. Hope I can make some time soon to rewrite it all and do it properly.

@fourmat
Copy link

fourmat commented Oct 16, 2020

Hi Maarten, I am also finding some curious results with this packing class. The concept is a lifesaver, but it's returning some unrealistic real-world box dimensions for a series of items that I have tested. I am trying to use this as an add-on to our current box packing system, as a way to handle products that are larger than the standard boxes we keep in inventory. The concept is to feed it all of the products that can't be placed in a standard box and calculate a "custom" box size that would handle them all. For instance, I had an order of QTY 25 - 66x6x3 and it's returning a container size of 66x6x75 with 25 levels, and a single box on item level.

I can also verify that it will return a different result if the dimensions are sorted differently. If I changed the dims to 66x3x6 it returns a 66x6x78 container with 11 levels with 2 boxes and 1 with 1 box. It seems to be using the height dimension and just stacking them straight up.

Feature request. To make this work in the real world, we would need to put some constraints on the resulting dimensions because it's obviously returning a container size that's too tall to ship.

I know you haven't had the opportunity to put much time into this project recently, but so far it is the only one that I have been able to find that tried to tackle this particular issue. I think it would be a benefit to a lot of people if it could be revisited.

@mdeboer mdeboer added the bug label Oct 20, 2020
@mdeboer mdeboer self-assigned this Oct 20, 2020
@mdeboer
Copy link
Member

mdeboer commented Oct 20, 2020

Thank you @fourmat! I'll try my best to find some time and look at this issue :D

@mdeboer
Copy link
Member

mdeboer commented Dec 27, 2021

I am working on a new version of this library and I'll make sure to fix this 😄

@mdeboer mdeboer added this to the v2.0 milestone Dec 27, 2021
@billj9000
Copy link

If it is of any help, I have an observation. I have been using V1.0 of this excellent project as the core of our shipping calculator for years (must be at least 8 years or more). I never realised it had been updated. I have never made any changes to this code because it seemed to work well.

It worked well for years but some time ago (perhaps a year or two) I started to notice the same sensitivity to order that the others have mentioned. As I say, I have not changed the code, nor have I made any changes to how the data is passed to it. What I have changed, however, is the version of PHP we are using. Initially we were using, I believe, PHP 5.6. We have moved up a couple of times until now we are using PHP 7.4. It may turn out not to be relevant but it might be a starting point to check PHP version compatibility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants