6
\$\begingroup\$

I've coded a function which uses a map and creates a random way between two points. Startpoint is always 0/0 and the goal is somewhere else everytime on the map.

Now I'd like to know if I could change anything to make that function more efficient or use another function.

But I need to create random ways and not the fastest one like with a A* algorithm.

func createWay(){
    setGoal()

    var found = false
    var targetX:Int!
    var targetY:Int!

    for y in 0..<currentSize{
        for x in 0..<currentSize{
            if map[x][y].name == goalItem.name{
                targetX = x
                targetY = y
            }
        }
    }

    while !found{
        tileOrder.removeAll(keepCapacity: false)
        countLabels.removeAll(keepCapacity: false)

        var walked = [[Int]](count: currentSize, repeatedValue: [Int](count: currentSize, repeatedValue: -1))

        var currentX = 0
        var currentY = 0

        tileOrder.append(map[currentX][currentY].name!.toInt()!)

        var up = true
        var left = true
        var right = true
        var down = true

        walked[0][0] = 1

        //Start counting at 2 because 1 is start-tile
        var counter = 2

        for tile in tileArray{
            tile.removeAllChildren()
        }

        while(true){
            //1 = left, 2 = right, 3 = up, 4 = down
            var direction:[Int] = []

            if currentX != 0 && walked[currentX-1][currentY] == -1{
                direction.append(1)
                left = true
            }else{
                left = false
            }
            if currentX != currentSize-1 && walked[currentX+1][currentY] == -1{
                direction.append(2)
                right = true
            }else{
                right = false
            }

            if currentY != currentSize-1 && walked[currentX][currentY+1] == -1{
                direction.append(3)
                up = true
            }else{
                up = false
            }

            if currentY != 0 && walked[currentX][currentY-1] == -1{
                direction.append(4)
                down = true
            }else{
                down = false
            }

            var move = false
            if direction.count == 0{
                break
            }
            var dire = direction.randomItem()
            switch dire{
            case 1:
                walked[currentX-1][currentY] = 1
                currentX--
                move = true
                left = true
                break
            case 2:
                walked[currentX+1][currentY] = 1
                currentX++
                move = true
                right = true
                break
            case 3:
                walked[currentX][currentY+1] = 1
                currentY++
                move = true
                up = true
                break
            case 4:
                walked[currentX][currentY-1] = 1
                currentY--
                move = true
                down = true
                break
            default:
                break
            }

            var label = XSKLabelNode(text: counter.description)
            var theNode = map[currentX][currentY]

            if move{
                label.fontName = font?.fontName
                label.alpha = 0
                label.fontSize = referenceNode.size.width/1.5
                label.setAlignment(XSKLabelNode.AlignmentMode.Center)

                var stillWorking = true

                countLabels.append(label)

                tileOrder.append(theNode.name!.toInt()!)

                theNode.addChild(label)
                counter++
            }
            println(currentSize*currentSize-1)
            if currentX == targetX && currentY == targetY && countLabels.count >= Int(Double(currentSize)*Double(currentSize)/2.5){
                label.fontSize = referenceNode.size.width/1.5
                label.text = (countLabels.count + 1).description
                found = true

                startLabel = XSKLabelNode(fontNamed: font?.fontName)
                startLabel.setAlignment(XSKLabelNode.AlignmentMode.Center)
                startLabel.text = "1"
                startLabel.alpha = 0
                startLabel.fontSize = referenceNode.size.width/1.5

                map[0][0].addChild(startLabel)
                break
            }
        }
    }
}
\$\endgroup\$
4
\$\begingroup\$

This method is way to big and there's way, way too much nesting going on.

You have

  • a massive if-else structure
  • a big switch statement
  • two big if statements

all inside a while(true) loop... which is so big it's impossible to find the break statements within, so the readability really, really suffers here.

This while loop as well as a for-in loop is also nested within another while loop.

Additionally, you have massively overloaded this with magic numbers.

When you leave comments like

//1 = left, 2 = right, 3 = up, 4 = down

It's a good sign that you need an enum:

enum Direction {
    case Left, Right, Up, Down
}

There's also a red flag here:

var targetX:Int!
var targetY:Int!

for y in 0..<currentSize{
    for x in 0..<currentSize{
        if map[x][y].name == goalItem.name{
            targetX = x
            targetY = y
        }
    }
}

What if that if never returns true? Now you're going to run into an error:

fatal error: unexpectedly found nil while unwrapping an optional

You need to decide what should happen when these values don't get set and be absolutely certain to handle that in some way that makes sense.

Beyond that, at this point, it's hard to really work out where the optimizations are. The code is too deeply nested to easily sort out what's going on. I can understand if you're having a tough time figuring out how to refactor this into more readable code--surely, that's part of why you're here. But in the meantime, your code is no where near self-documenting, and you don't have enough comments to make up for its lack of self documentation.

\$\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.