10
\$\begingroup\$
<!DOCTYPE html>
<html>
<script>

  ////////////////////////////    
 // Code by Zachary Holmes //
////////////////////////////

// This program determines the number of boxes that can fit into the back of a truck

// Declaring box size in centimetres, and converting it into metres
var boxHeight = prompt("What is the height of your box in centimetres?") / 100;
var boxWidth = prompt("What is the width of your box in centimetres?") / 100;
var boxLength = prompt("What is the length of your box in centimetres?") / 100;

// Declaring truck size in metres
var truckHeight = prompt("What is the height of your truck in metres?");
var truckWidth = prompt("What is the width of your truck in metres?");
var truckLength = prompt("What is the length of your truck in metres?");

// Declaring variables used in equation
var totalHeight = (truckHeight / boxHeight) | 0;
var totalWidth = (truckWidth / boxWidth) | 0;
var totalLength = (truckLength / boxLength) | 0;

// Calculations
var boxNumber = totalHeight * totalWidth * totalLength;

// Output answer
if (boxNumber = 1)
 {
  alert("You can fit up to" + " " + boxNumber + " " + "box into the truck");
 }
else
 {
  alert("You can fit up to" + " " + boxNumber + " " + "boxes into the truck");  
 }

</script>
</html>
\$\endgroup\$
  • 6
    \$\begingroup\$ Welcome to Code Review! What are you looking for with this post? Style recommendations? Performance tweaks? Having a question (or several questions) we can respond to goes a long way towards getting a good code review. \$\endgroup\$ – BeetDemGuise May 21 '14 at 18:40
  • \$\begingroup\$ super simple code, and it looks good. you can definitely add to this though. \$\endgroup\$ – Malachi May 21 '14 at 18:48
  • \$\begingroup\$ you should add more functionality to your code and post a follow up question, add some input checking, to make sure the user enters in real information. maybe add some looping and Decision making logic (more than what you have) \$\endgroup\$ – Malachi May 21 '14 at 19:16
  • \$\begingroup\$ Is there any way for me to check if the value of a variable is numeric? \$\endgroup\$ – DatLumberZach May 21 '14 at 23:39
  • \$\begingroup\$ I posted a followup Q: \$\endgroup\$ – DatLumberZach May 22 '14 at 0:48
11
\$\begingroup\$

You have a bug in the output: your whole program always does the equivalent of

alert("You can fit up to 1 box into the truck");

One way to prevent that kind of error is to use "Yoda conditions", which put the constant first:

if (1 == boxCount)

I think boxCount is a slightly better name than boxNumber, which sounds like it could be referring to the nth box.

Your if-else brace style is probably one of the rarest and least favoured ones possible. The braces, being aligned with neither the outer level nor the inner level, create visual clutter.

I'd actually prefer to use a ternary expression instead of repeating the entire statement:

alert("You can fit up to " +
      boxCount + (1 == boxCount ? " box" : " boxes") +
      " into the truck");

totalHeight, totalWidth, totalLength are misnamed, in my opinion. You're not totalling anything. Furthermore, the values that you store in them are not lengths (with metres as the unit), but unitless numbers. I think heightMultiples, widthMultiples, and lengthMultiples would be more appropriate.

In real life, the problem that you are trying to solve is trickier than that. You have the two options for orienting each box, assuming that the boxes need to stay upright. If you are allowed to tip boxes over on their side, then you have six possible orientations altogether. You would have to try all allowable orientations for optimal packing.

On the other hand, if the boxes are strapped to pallets, you may have no choice at all in the orientation, due to the way the forklift needs to enter the truck.

Fortunately, packing rectangular prisms is a relatively simple problem, compared to other objects. ☺

\$\endgroup\$
  • \$\begingroup\$ To flesh this out, if your boxes are just slightly off square, a 90 degree rotation can sometimes help you fit a lot more in. Assuming a 102" x 53' trailer and 10.2" by 10.21" boxes, you could fit 10 boxes across and 62 lengthwise by having the slightly longer side lengthwise, but only 9 across and still 63 lengthwise in the other orientation. At the other extreme, imagine a 96" x 6" box in the same trailer - with the box's long side across the trailer you can fit 1 across and 106 lengthwise, but then you could add another 6 boxes in the other orientation in the gap at the side of the trailer. \$\endgroup\$ – MattW May 22 '14 at 16:42
7
\$\begingroup\$

I guess I would change your box variable names to something a little more descriptive maybe something like:

 var boxHeightInMeters;

Like I said in the comments, this is a super simple script that doesn't really do a whole lot, there isn't much to tweak really.


Maybe this to make sure that you don't get divide-by-zero errors:

var totalHeight = boxHeight > 0 ? (truckHeight / boxHeight) : 0;

The statement above is called a ternary statement,

Conditional Statement ? {true} : {false}

You don't want to divide by 0 anywhere.

\$\endgroup\$
  • \$\begingroup\$ Since they both are in metres (after conversions at least) I believe that that would be redundant \$\endgroup\$ – DatLumberZach May 22 '14 at 0:54
  • \$\begingroup\$ the code doesn't say that though. your code should tell a story, comments are like a narrator that give a slice of background around something that doesn't seem to make sense if you don't know the code. you ask for Centimeters and then divide by 100, someone may ask why? it's obvious to us because 100 centimeters go into a meter and the other measurements are being asked for in Meters. it's slightly obscure is what I am trying to say \$\endgroup\$ – Malachi May 22 '14 at 13:32
5
\$\begingroup\$

Fun question,

there is a lot of repeated code there to get the size of things, I would propose you use a re-usable function for this, something like this:

function getDimensions( thing , unit ){
  var dimensions = {};
  dimensions.height = prompt('What is the height of your ' + thing + ' in ' + unit + '?');
  dimensions.width  = prompt('What is the width of your ' + thing + ' in ' + unit + '?');
  dimensions.length = prompt('What is the length of your ' + thing + ' in ' + unit + '?');
  return dimensions;                           
}

Furthermore, to generalize a bit, but also to remove repetition, you might want a mechanism to convert the measures from one unit to another:

function convertCentimerToMeter( length ){
  return length / 100;
}

function convertDimensions( dimensions, converter ){
  dimensions.height = converter( dimensions.height );
  dimensions.width  = converter( dimensions.width );
  dimensions.length = converter( dimensions.length );
}

convertDimensions( box , convertCentimerToMeter );

This would convert the dimensions of a box from centimers to meters.

The output UI I would separate from the rest of the code, plus I would have some deeper thoughts about which parts are copy pasted and which parts are unique in, something like this:

function displayPackingCount( boxCount ){
  var s = boxCount + ' box';
  if( boxCount > 1 ){
    s = s + 'es';
  }
  alert('You can fit up to ' + s + ' into the truck');
}

You will note that I made 'box' a parameter string for getDimensions and hardcoded it in displayPackingCount, I will leave it to you whether you want to parametrize in displayPackingCount as well.

Finally, the calculation would benefit of it's own function as well:

function calculateOptimalPacking( container , thing ){
  return ( container.width  / thing.width  | 0 ) * 
         ( container.height / thing.height | 0 ) *
         ( container.length / thing.length | 0 );
}

Again, I tried to write this function more generically.

Altogether that gives something like

// This script determines the number of boxes that can fit into the back of a truck
function getDimensions( thing , unit ){
  var dimensions = {};
  dimensions.height = prompt('What is the height of your ' + thing + ' in ' + unit + '?');
  dimensions.width  = prompt('What is the width of your ' + thing + ' in ' + unit + '?');
  dimensions.length = prompt('What is the length of your ' + thing + ' in ' + unit + '?');
  return dimensions;                           
}

function convertCentimerToMeter( length ){
  return length / 100;
}

function convertDimensions( dimensions, converter ){
  dimensions.height = converter( dimensions.height );
  dimensions.width  = converter( dimensions.width );
  dimensions.length = converter( dimensions.length );
}

function calculateOptimalPacking( container , thing ){
  return ( container.width  / thing.width  | 0 ) * 
         ( container.height / thing.height | 0 ) *
         ( container.length / thing.length | 0 );
}

function displayPackingCount( boxCount ){
  var s = boxCount + ' box';
  if( boxCount > 1 ){
    s = s + 'es';
  }
  alert('You can fit up to ' + s + ' into the truck');
}


var box   = getDimensions( 'box' , 'centimetres' );
var truck = getDimensions( 'truck' , 'meters' );
convertDimensions( box , convertCentimerToMeter );
var boxCount = calculateOptimalPacking( truck , box );
diplayPackingCount( boxCount );
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.