Find common directory path

Question

I answered the problem of Find common directory path from Rosetta Code.

The problem consists of passing '/home/user1/tmp/coverage/test', '/home/user1/tmp/covert/operator', and '/home/user1/tmp/coven/members' to a function, the resultant path should be the valid directory '/home/user1/tmp' and not the longest common string '/home/user1/tmp/cove'.

The solution (the working code) is:

import Foundation
func getPrefix(_ text:[String]) -> String? {
 var common:String = text[0]
 for i in text {
 common = i.commonPrefix(with: common)
 }
 return common
}
var test = ["/home/user1/tmp/coverage/test", 
 "/home/user1/tmp/covert/operator",
 "/home/user1/tmp/coven/members"]
var output:String = getPrefix(test)!
print(output)

Could the above code be further optimized?

• Eric Aya: Looks to me that if your code doesn't return the right result, it doesn't need to be optimized but to be fixed: that would make your question off-topic for Code Review and you would have to ask that on Stack Overflow instead.
• lf_araujo: I don't understand, the code works! What do you mean?
• Eric Aya: The code runs but doesn't resurn the right result. I think this question belongs on Stack Overflow.
• lf_araujo: The result should be /home/user1/tmp, and this is what I have in my terminal. What do you get? I am running it on the REPL.
• Eric Aya: Ah! I get /home/user1/tmp/cove. See my screenshot of my Playground.

Answer 1

Naming

Good names can greatly improve the readability of a program. Many of the names in the posted code can be improved:

• getPrefix: the function returns the common prefix. It would be good to include that important clarifying detail in the name.
  • If you intend to return the common parent directory, I would call the function getCommonParent.
• text: the parameter is not just any text, it's an array of paths, so paths would be a better name that conveys the intention.
• i: is fine in simple counting loops, but when you iterate over values that have a meaningful name, it's better to use that, for example in this example path.
• test is also an overly generic name, instead of the more descriptive paths

Algorithm

The implementation will call commonPrefix method text.count + 1 times. This is probably fine, but you could reduce the number of calls to commonPrefix by using a divide and conquer strategy.

For example, consider you have paths p1, p2, ..., pn. You could call commonPrefix for each pair, to get n / 2 results. Then you could call commonPrefix for each of those pairs. And so on, until there is only one pair left:

p1 p2 p3 p4 p5 p6 p7 p8
|__| |__| |__| |__|
 |_____| |_____|
 |___________|
 |
 = the common prefix

Admittedly, this might be overkill for your use case.