1

AS3's got some awkward rules about variable scope, due to its use of hoisting. I don't like pointlessly leaving a bunch of compiler warnings lying around, but it's more important to me for my code to readable and properly written, regardless. This produces a couple of duplicate variable definition warnings:

    private function arrangeElementsLine(pElementSize:Dimensions):void
    {
        if (m_eOrientation == HORIZONTAL)
        {
            for (var i:uint = 0; i < target.numElements; i++)
            {
                var layoutElement:ILayoutElement = target.getElementAt(i);
                layoutElement.setLayoutBoundsSize(pElementSize.x, pElementSize.y);
                layoutElement.setLayoutBoundsPosition(pElementSize.x * i, 0);
            }
        }
        else
        {
            for (var i:uint = 0; i < target.numElements; i++)
            {
                var layoutElement:ILayoutElement = target.getElementAt(i);
                layoutElement.setLayoutBoundsSize(pElementSize.x, pElementSize.y);
                layoutElement.setLayoutBoundsPosition(0, pElementSize.y * i);
            }
        }
    }

Is it generally considered to be better and more conventional to remove the redundant var and :<type> syntax for i and layoutElement in the second for loop, or is it more proper to just leave them in there (due to the way their usage is isolated to those two for loops)? Thanks!

EDIT

This was asked a while back, but when I asked this, part of what I was working on was making something run more efficiently; so I put if outside of any for statements and avoided method calls inside the for statements.

  • the way i usually solved this was by putting the different loops into their own methods – Lucina Oct 7 '13 at 19:46
2

Instead of using the same variable, make them unique by putting more of their purpose into their names:

var horizontalLayoutElement:ILayoutElement = target.getElementAt(i);
...
var verticalLayoutElement:ILayoutElement = target.getElementAt(i);

or if you don't like such long variable names:

var hLayoutElement:ILayoutElement = target.getElementAt(i);
...
var vLayoutElement:ILayoutElement = target.getElementAt(i);

You can also emulate hoisting by putting all your variables declarations at the top of your functions, but since this makes code much harder to read due to the distance in code between declaration and usage, I would avoid it.

1

You have a lot of duplicate code here, which defeats the DRY principle. This would be better:

private function arrangeElementsLine(pElementSize:Dimensions):void {
    for (var i:uint = 0; i < target.numElements; i++) {
        var layoutElement:ILayoutElement = target.getElementAt(i);
        layoutElement.setLayoutBoundsSize(pElementSize.x, pElementSize.y);

        if (m_eOrientation == HORIZONTAL) {
            layoutElement.setLayoutBoundsPosition(pElementSize.x * i, 0);
        } else {
            layoutElement.setLayoutBoundsPosition(0, pElementSize.y * i);
        }
    }
}

This also solves your issues with variable duplication.

  • Thanks! This was a while back, but part of what I was doing when I asked this was to try to make something run more efficiently. So I put the if statement around two fors and avoided making function calls inside the fors. – Panzercrisis Dec 22 '14 at 13:35
0

Personally, I prefer reuse the sames variables when they are exactly the same. As the content is the same and the different loop don't stuck each other by overriding the variables, reusing the variables will resolve your problem of warning without making the code less readable, and, in case of pure optimization, it will take less memory (even if 4 bytes is nothing, it's 4 bytes lighter anyway). So, personally, I prefer using a function like that:

private function arrangeElementsLine(pElementSize:Dimensions):void
{
    var i:uint;
    var layoutElement:ILayoutElement;

    if (m_eOrientation == HORIZONTAL)
    {
        for (i = 0; i < target.numElements; i++)
        {
            layoutElement = target.getElementAt(i);
            layoutElement.setLayoutBoundsSize(pElementSize.x, pElementSize.y);
            layoutElement.setLayoutBoundsPosition(pElementSize.x * i, 0);
        }
    }
    else
    {
        for (i = 0; i < target.numElements; i++)
        {
            layoutElement = target.getElementAt(i);
            layoutElement.setLayoutBoundsSize(pElementSize.x, pElementSize.y);
            layoutElement.setLayoutBoundsPosition(0, pElementSize.y * i);
        }
    }
}

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.