The simplest method of refactoring and maybe the most used is performed by moving the code around.
We can move it from one function to another, from one class to another or create a new entity, in our case, a new function.
Consider the following function to validate three fields: first name, last name and address. Each variable needs to have a value.
var firstName, lastName, address : String?
firstName = "John"
lastName = "Smith"
address = "20 New Way St."
func validateForm() -> Bool {
var valid = true
if (firstName == nil) || (firstName!.characters.count == 0) {
valid = false
}
if (lastName == nil) || (lastName!.characters.count == 0) {
valid = false
}
if (address == nil) || (lastName!.characters.count == 0) {
valid = false
}
return valid
}
While the code is just fine for a short function what if there are more fields? Also what if the rule change, for example each item should have at least two characters?
We easily notice that the some code is repeated. This was probably done by copy paste. What we might not have noticed is that we have a bug:
if (address == nil) || (lastName!.characters.count == 0) {
valid = false
}
I am sure this happened to you too when programming by "copy/paste".
We'll extract the duplicated code as a separate function:
func validateItem(item:String?) -> Bool{
if (item == nil) || (item!.characters.count == 0) {
return false
}
return true
}
Our function becomes:
func validateForm() -> Bool {
var valid = true
if !validateItem(firstName) {
valid = false
}
if !validateItem(lastName) {
valid = false
}
if !validateItem(address) {
valid = false
}
return valid
}
Assuming the validation rules change and the minimum length for each item is two, we need to change just validateItem function:
func validateItem(item:String?) -> Bool{
if (item == nil) || (item!.characters.count < 2) {
return false
}
return true
}
I think the function "validateItem" should have a more specific name. If I read the function "validateForm", I have no idea what "validateItem" does without looking at its source code. Also, what is an item? The function is very specific, because it takes an argument of type String?. So the item is the string then. Should it have been named "validateString" then? I think so.
ReplyDeleteFunctions should be named to reflect their purpose as best as possible. The goal is to express the purpose of the function through its name.
good point
Delete