Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I wrote the following code to give an HTML element max width/height in its parent container. I think I address the box model issues but I am wondering if anyone can see issues and or shortcomings.

Are there other solutions out there that accomplish this? I want to make sure I didn't re-invent the wheel.

function heightBuffer(control) {
    return parseInt((control.outerHeight(true)) - control.height());
}

function widthBuffer(control) {
    return parseInt((control.outerWidth(true)) - parseInt(control.width()));
}

function MaxHeightInParent(control, minHeight, controlHeightsToSubtract) {
    var h = parseInt(control.parent().height()) - heightBuffer(control);
    if (controlHeightsToSubtract != null)
        $.each(controlHeightsToSubtract, function(index, value) {
            h = h - parseInt(value.outerHeight(true));
        });

    if (minHeight != null && h < minHeight) h = minHeight;
    control.height(0);
    control.css('min-height', h);
}

function MaxWidthInParent(control, minWidth, controlWidthsToSubtract) {
    var w = parseInt(control.parent().width()) - widthBuffer(control);
    if (controlWidthsToSubtract != null)
        $.each(controlWidthsToSubtract, function(index, value) {
            w = w - parseInt(value.outerWidth(true));
        });
    if (minWidth != null && w < minWidth) w = minWidth;
    control.width(0);
    control.css('min-width', w);
}

Note controlHeightsToSubtract / controlWidthsToSubtract are if you wish to pass an array of controls that share the containing element with the element you are attempting to maximize in Height/Width.

share|improve this question
add comment

1 Answer

up vote 1 down vote accepted

What is

parseInt((control.outerHeight(true)) - control.height());

supposed to achieve?

The result of - is always going to be a number, so what this does is convert the number to a string and back to a number. This can only lose precision and waste time.

Similarly, the use of parseInt on the first argument is also unnecessary

parseInt(control.parent().height()) - heightBuffer(control)

since

"4" - 1 === 3

In fact, the automatic conversion that - does is better than that done by parseInt since parseInt falls back to octal on some interpreters but not others.

"10"             - 1 === 9
parseInt("10")   - 1 === 9
"0x10"           - 1 === 15
parseInt("0x10") - 1 === 15
"010"            - 1        // throws reliably
parseInt("010")  - 1 === 7  // on some and 9 on others

so I'd get rid of all the uses of parseInt as an operand to -.

You seem to be setting min-height but the method is called MaxHeight. That confuses me.

When you change the CSS

control.css('min-height', h)

you might want to specify units as in

control.css('min-height', h + "px")

I'm not sure how you're handling the widths of margins and borders on the controls. Is that important to you?

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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