Tuesday, March 8, 2016

Refactoring - remove control flag

Many times when there are a series of conditional tests, we use a flag to keep a temporary boolean value.

If this is happening inside a function we can replace this flag with return or exit statement.

Have a look at the following example:

class User {
    var firstName = ""
    var middleName = ""
    var lastName = ""
}

We also have a database of users:

var userArray = [User]()

And here is a search function for a user that has a specific name:

func searchName(name:String) -> User? {
    var found = false
    var user:User?

    for var index = 0; (index < userArray.count) && !found; ++index {
        
        let crtUser = userArray[index]
        
        if crtUser.firstName == name {
            user = crtUser
            found = true
        }
        
        if crtUser.middleName == name {
            user = crtUser
            found = true
        }
        
        if crtUser.lastName == name {
            user = crtUser
            found = true
        }
    }
    return user
}

So many lines for a simple search.

We can eliminate the variable and instead of storing locally the user, we can just return it as follows:

func searchName(name:String) -> User? {
    
    for var index = 0; index < userArray.count; ++index {
        
        let crtUser = userArray[index]
        
        if crtUser.firstName == name {
            return crtUser
        }
        
        if crtUser.middleName == name {
            return crtUser
        }
        
        if crtUser.lastName == name {
            return crtUser
        }
    }
    return nil
}

We can continue the refactoring by combining all the tests in one:

func searchName(name:String) -> User? {
    
    for var index = 0; index < userArray.count; ++index {
        
        let crtUser = userArray[index]
        
        if (crtUser.firstName == name) || (crtUser.middleName == name) || (crtUser.lastName == name) {
            return crtUser
        }
    }
    return nil
}

Also we notice that a for-in might work better and the code look more elegant:

func searchName(name:String) -> User? {
    
    for crtUser in userArray {
        
        if (crtUser.firstName == name) || (crtUser.middleName == name) || (crtUser.lastName == name) {
                return crtUser
        }
    }
    return nil
}


No comments:

Post a Comment