Рефакторинг метода, содержащего условные выражения с крайне разными одинаковыми блоками кода;) - PullRequest
0 голосов
/ 04 декабря 2009

Итак, у меня есть этот вонючий метод, два условных блока делают почти одно и то же, но с радикально разными параметрами (по крайней мере, на мой взгляд). Я хочу почистить его в стиле дяди Боба, но не могу придумать, как сделать это аккуратно. Итак, я прихожу к вам, мои сказочные друзья-ботаники, чтобы посмотреть, как вы можете отогнать это до чего-то, что не заставляет человека выбивать им глаза. Код AS3, но, на мой взгляд, это не имеет значения.

/**
 * Splits this group into two groups based on the intersection of the group
 * with another group. The group is split in a direction to fill empty
 * cells left by the splitting group.
 *
 * @param onGroup
 * @param directionToMoveSplitCells
 * @return
 *
 */
public function split(onGroup:CellGroup, directionToMoveSplitCells:String):CellGroup
{
    if (!hasIntersection(onGroup))
        return this;
    var numCellsToSplit:int = 0;
    var splitCells:Array;
    var newGroup:CellGroup;
    var numberOfCellsToSplit:int;
    var splitStartIndex:int;
    var resultingGroupStartIndex:int;

    if (directionToMoveSplitCells == "RIGHT")
    {
        numberOfCellsToSplit = endIndex - onGroup.startIndex + 1;
        splitStartIndex = length - numberOfCellsToSplit;
        splitCells = trimCells(splitStartIndex, numberOfCellsToSplit);
        resultingGroupStartIndex = onGroup.endIndex + 1;

        if (splitCells.length > 0)
        {
            newGroup = row.createGroup(splitCells, resultingGroupStartIndex)
            newGroup.nextGroup = nextGroup;
            if (newGroup.nextGroup)
                newGroup.nextGroup.previousGroup = newGroup;
            newGroup.previousGroup = this;
            nextGroup = newGroup;

        }
    }
    else
    {
        numberOfCellsToSplit = onGroup.endIndex - startIndex + 1;
        splitStartIndex = 0;
        splitCells = trimCells(splitStartIndex, numberOfCellsToSplit);
        resultingGroupStartIndex = onGroup.startIndex - splitCells.length;

        if (splitCells.length > 0)
        {
            newGroup = row.createGroup(splitCells, resultingGroupStartIndex)
            newGroup.previousGroup = previousGroup;
            if (newGroup.previousGroup)
                newGroup.previousGroup.nextGroup = newGroup
            previousGroup = newGroup;
            newGroup.nextGroup = this;
            var newX:int = (onGroup.endIndex + 1) * cellSize.width;
            x = newX;
        }

    }

    removeArrayOfCellsFromGroup(splitCells);
    row.joinGroups();
    row.updateGroupIndices();
    repositionCellsInGroup();

    return newGroup;
}

Ответы [ 4 ]

1 голос
/ 04 декабря 2009
public function split(onGroup:CellGroup, directionToMoveSplitCells:String):CellGroup
{
    if (!hasIntersection(onGroup))
        return this;

    var newGroup:CellGroup;

    if (directionToMoveSplitCells == "RIGHT")
    {
        newGroup = splitGroupAndMoveSplitCellsRight(onGroup);
        if (!newGroup)
            return;

        insertAsNextGroupInLinkedList(newGroup);
    }
    else
    {
        newGroup = splitGroupAndMoveSplitCellsLeft(onGroup);
        if (!newGroup)
            return;

        insertAsPreviousGroupInLinkedList(newGroup);

        x = (onGroup.endIndex + 1) * cellSize.width;
    }

    removeArrayOfCellsFromGroup(splitCells);
    row.joinGroups();
    row.updateGroupIndices();
    repositionCellsInGroup();

    return newGroup;
}


private function splitGroupAndMoveSplitCellsRight(onGroup:CellGroup):CellGroup
{
    var numCellsToSplit:int = endIndex - onGroup.startIndex + 1;
    var splitStartIndex:int = length - numberOfCellsToSplit;

    var splitCells:Array = trimCells(splitStartIndex, numberOfCellsToSplit);
    if (!splitCells.length)
        return null;

    var resultingGroupStartIndex:int = onGroup.endIndex + 1;

    return row.createGroup(splitCells, resultingGroupStartIndex);
}

private function splitGroupAndMoveSplitCellsLeft(onGroup:CellGroup):CellGroup
{
    var numCellsToSplit:int = onGroup.endIndex - startIndex + 1;
    var splitStartIndex:int = 0;

    var splitCells:Array = trimCells(splitStartIndex, numberOfCellsToSplit);
    if (!splitCells.length)
        return null;

    var resultingGroupStartIndex:int = onGroup.startIndex - splitCells.length;

    return row.createGroup(splitCells, resultingGroupStartIndex);
}

private function insertAsNextGroupInLinkedList(group:CellGroup):void
{
    var currentNextGroup:CellGroup = nextGroup;
    if (currentNextGroup)
    {
        group.nextGroup = currentNextGroup;
        currentNextGroup.previousGroup = group;
    }

    group.previousGroup = this;
    nextGroup = group;
}

private function insertAsPreviousGroupInLinkedList(group:CellGroup):void
{
    var currentPreviousGroup:CellGroup = previousGroup;
    if (currentPreviousGroup)
    {
        group.previousGroup = currentPreviousGroup;
        currentPreviousGroup.nextGroup = group;
    }

    group.nextGroup = this;
    previousGroup = group;
}
1 голос
/ 04 декабря 2009
/**
 * Splits this group into two groups based on the intersection of the group
 * with another group. The group is split in a direction to fill empty
 * cells left by the splitting group.
 *
 * @param onGroup
 * @param directionToMoveSplitCells
 * @return
 *
 */
public function split(onGroup:CellGroup, directionToMoveSplitCells:String):CellGroup
{
        if(!hasIntersection(onGroup)) return this;
        var numCellsToSplit:int = 0;
        var splitCells:Array;
        var newGroup:CellGroup;
        var numberOfCellsToSplit:int;
        var splitStartIndex:int;
        var resultingGroupStartIndex:int;

  numberOfCellsToSplit = (directionToMoveSplitCells == "RIGHT" ? (endIndex - onGroup.startIndex) : (onGroup.endIndex - startIndex)) + 1;
  splitStartIndex = directionToMoveSplitCells == "RIGHT" ? (length - numberOfCellsToSplit) : 0;
  splitCells = trimCells(splitStartIndex, numberOfCellsToSplit);
  resultingGroupStartIndex = directionToMoveSplitCells == "RIGHT" ? (onGroup.startIndex - splitCells.length) : (onGroup.endIndex + 1);

  if (splitCells.length > 0)
        {
                newGroup = row.createGroup(splitCells, resultingGroupStartIndex)
                newGroup.nextGroup = nextGroup; //not sure how to not set this from jump
                newGroup.previousGroup = previousGroup; //not sure how to not set this from jump
                if (newGroup.previousGroup){
     newGroup.previousGroup.nextGroup = newGroup;
     previousGroup = newGroup;
     var newX:int = (onGroup.endIndex + 1) * cellSize.width;
                 x = newX;
    }
                if (newGroup.nextGroup) newGroup.nextGroup.previousGroup = newGroup;
    else{
     newGroup.nextGroup = this;
     newGroup.previousGroup = this;
                 nextGroup = newGroup;
    }
        }

        removeArrayOfCellsFromGroup(splitCells);
        row.joinGroups();
        row.updateGroupIndices();
        repositionCellsInGroup();

        return newGroup;
}
1 голос
/ 04 декабря 2009

Это все, что происходит со мной. Связанный список на самом деле является отдельной деталью ... так что, возможно, его можно реорганизовать ....

    public function split(onGroup:CellGroup, directionToMoveSplitCells:String):CellGroup
{
        if (!hasIntersection(onGroup))
                return this;
        valr splitCells:Array;
        var newGroup:CellGroup ;
        var numberOfCellsToSplit:int;
        var splitStartIndex:int;
        var resultingGroupStartIndex:int;

        if (directionToMoveSplitCells == "RIGHT")
        {
                numberOfCellsToSplit = this.endIndex - onGroup.startIndex + 1;
                splitStartIndex = this.length - numberOfCellsToSplit;
        splitCells = trimCells(splitStartIndex, numberOfCellsToSplit);
                resultingGroupStartIndex = onGroup.endIndex + 1;

                if (splitCells.length > 0)
                {
                        newGroup = row.createGroup(splitCells, resultingGroupStartIndex);
                        nextGroup=insertGroup(newGroup,this,nextGroup);
                }
        }
        else
        {
                numberOfCellsToSplit = onGroup.endIndex - startIndex + 1;
                splitStartIndex = 0;
        splitCells = trimCells(splitStartIndex, numberOfCellsToSplit);
                resultingGroupStartIndex = onGroup.startIndex - splitCells.length;

                if (splitCells.length > 0)
                {
                        newGroup = row.createGroup(splitCells, resultingGroupStartIndex);
                        previousGroup=insertGroup(newGroup,previousGroup,this);
                        var newX:int = (onGroup.endIndex + 1) * cellSize.width;
                        x = newX;
                }

        }

        removeArrayOfCellsFromGroup(splitCells);
        row.joinGroups();
        row.updateGroupIndices();
        repositionCellsInGroup();

        return newGroup;
}

private function insertGroup(toInsert:CellGroup,prior:CellGroup,next:CellGroup):CellGroup
{
    toInsert.nextGroup = next;
    toInsert.previousGroup = prior;
    if (toInsert.nextGroup )
            toInsert.nextGroup.previousGroup = toInsert;
    if (toInsert.previousGroup )
        toInsert.previousGroup.nextGroup = toInsert;
    return toInsert;
}

Мое недоставление назначения splitCells состоит в том, чтобы указать, что это - безусловная безусловная строка в блоке. Я смотрел на то, что делал Anon, но я не вижу способа улучшить код таким образом.

0 голосов
/ 04 декабря 2009
var groupThatWillReceiveCells = this;
var groupThatWontReceiveCells = onGroup;

if (directionToMoveSplitCells == "RIGHT")
{
    groupThatWillReceiveCells = onGroup;
    groupThatWontReceiveCells = this;
}

Переименуйте материал по мере необходимости.

...