I have been refactoring with my seniors at a company that joined a new graduate in April of this year, so I will write an article in the form of a series of what I learned about refactoring there.
This time, I will explain the refactoring of the long method.
If there is something wrong, I will be happy to cry if you point it out mm
--scala (sometimes java)
As the name implies, the method contains too much code.
Why is it the problem that there is too much code in the method in the first place?
There are two possible points.
For example, if you write about 100 lines of code for the same method, you will give up reading it in about the third line.
If you are doing multiple processes in the same function, UT is difficult, so it becomes a hotbed of bugs at the moment when it is not covered.
In the following example, in the createUrl method, there are two processes, one is to get the value from redis and the other is to actually create the URL.
def createUrl(key: String): String = {
val keyword = redisClient.getCaches(key).orElse("hoge")
s"http://localhost:9000?q=${keyword}"
}
So why is the function so long?
There are two main possible causes:
If you eliminate the two causes listed above one by one, the code smell of the long method is likely to disappear. There are many ways to solve the long method, so which method to apply depends on the long method you are hitting. So, this is all! !! !! I can't say that
In general, I think that it can be solved by doing the following one as a big concept of the solution method of the long method.
There are many ways to think of extracting this function.
The processing in the method is complicated, and since the specific processing is written only in the if, the abstraction level is not uniform, so it is difficult to read.
def search(searchResult: SearchResult): String = {
if(searchResult.totalHits > 0 || searchResult.result.nonEmpty) {
Ok(
createResponse(.....)
)
} else {
NotFound
}
}
If you bring the contents of if to a private method, the abstraction level in the method will be aligned and it will be easier to read.
def search(searchResult: SearchResult): String = {
if(existSearchResult(searchResult)) {
Ok(
createResponse(.....)
)
} else {
NotFound
}
}
private def existSearchResult(searchResult: SearchResult): Boolean = {
searchResult.totalHits > 0 || searchResult.result.nonEmpty
}
To further refactor the previous example in terms of cohesion.
def search(searchResult: SearchResult): String = {
if(searchResult.hasContent) {
Ok(
createResponse(.....)
)
} else {
NotFound
}
}
case class SearchResult(
totalHits: Long,
result: Option[Result]
) {
def hasContent: Boolean = totalHits > 0 || result.isDefined
}
SearchResult has become more cohesive, making it even easier to read.
Cohesion will be discussed in the next large class refactoring article.
In the first example, multiple processes are performed in one method.
def createUrl(key: String): String = {
val keyword = redisClient.getCaches(key).orElse("hoge")
s"http://localhost:9000?q=${keyword}"
}
The process of creating a URL and the process of getting a keyword from redis are completely separated. Instead of calling getKeyWord in the createUrl method The caller of each method (main method in this case) calls each method.
By doing so, the abstraction of the process will be uniform and it will be much easier to read.
def main() {
val keyword = getKeyWord(key)
createUrl(keyword)
}
private getKeyWord(key: String): String = {
redisClient.getCaches(key).orElse("hoge")
}
private def createUrl(key: String): String = {
s"http://localhost:9000?q=${keyword}"
}
This time, I introduced an example of refactoring related to the long method.
If you are aware of the following two points, you will inevitably be able to define methods without smell.
--One method does one process
--Match the abstraction of the processing in the function.
Thank you until the end.
Next time, I will write a refactoring explanation about the large class.
Then.