Extract regex matches with non matching parts

Question

I want to check a string by using multiple search patterns and split the string at every occurrence. I'm using RegEx and it works fine. But I'm worrying about efficiency and performance.

struct StringPart {
    let text    : String
    var matched : Int   // index of pattern
    let pos     : Int

    init(searchText: String, matched : Int, firstIndex : Int) {
        self.text    = searchText
        self.matched = matched
        self.pos     = firstIndex
    }
}

extension StringPart {
    func toString() -> String {
        return text
    }
}

func SplitNumsArray(searchString : String, patterns : [String]) -> [StringPart] {
    var results = [StringPart]()
    var index   = 0

    // init with first string
    results.append(StringPart(searchText: searchString, matched: -1, firstIndex: 0))

    index += searchString.characters.count

    var patternIndex = 0
    for pattern in patterns {

        var resultIndex = 0
        for result in results {
            if result.matched == -1 {
                let res1Run = SplitNums(searchText : result.text, pattern : pattern, patternIndex: patternIndex)
                results.remove(at: resultIndex)
                results.insert(contentsOf: res1Run, at: resultIndex)
                resultIndex += res1Run.count - 1
            }
            resultIndex += 1
        }

        patternIndex += 1
    }

    return results
}

func SplitNums(searchText : String, pattern : String, patternIndex : Int = 0) -> [StringPart] {
    var results = [StringPart]()
    var index = 0

    var s = searchText

    results.append(StringPart(searchText: s, matched: -1, firstIndex: index))
    index += s.characters.count

    let regex = try! NSRegularExpression(pattern: pattern, options: [])

    // initial search
    var matches = regex.matches(in: s, options: [], range: NSRange(location: 0, length: s.characters.count))

    while matches.count > 0 {
        // we only need the first one!
        let match = matches[0]
        if match.numberOfRanges > 0 {
            let range = match.rangeAt(0)

            let beforeRange = s.startIndex..<s.characters.index(s.startIndex, offsetBy: range.location)
            let foundRange = s.characters.index(s.startIndex, offsetBy: range.location)..<s.characters.index(s.startIndex, offsetBy: range.location+range.length)
            let endRange = s.characters.index(s.startIndex, offsetBy: range.location+range.length)..<s.endIndex

            let foundString  = s.substring(with: foundRange)
            let beforeString = s.substring(with: beforeRange)
            let endString    = s.substring(with: endRange)

            //  remove last part, because it will be replaced by new split
            let lastIndex = results.count - 1
            results.remove(at: lastIndex)

            if beforeString.characters.count > 0 {
                results.append(StringPart(searchText: beforeString, matched: -1,           firstIndex: index))
                index += beforeString.characters.count
            }
            if foundString.characters.count > 0 {
                results.append(StringPart(searchText: foundString,  matched: patternIndex, firstIndex: index))
                index += foundString.characters.count
            }
            if endString.characters.count > 0 {
                results.append(StringPart(searchText: endString,    matched: -1,           firstIndex: index))
                index += endString.characters.count
            }

            s = endString   // last part can be reused for next search
        }

        // read again!
        matches = regex.matches(in: s, options: [], range: NSRange(location: 0, length: s.characters.count))
    }

    return results
}

There are two functions: one handles one pattern string, and the other one is calling the first one repeatedly. But in one call I get several matching information. I drop it and use the remaining string again for the next matching. This works fine, but I redo something, which was already done.

Some examples:

let numberPatterns =  [  "\\d{4}\\.\\d{2}\\.\\d{2}-\\d{2}\\.\\d{2}\\.\\d{2}_.*_",
                         "\\d{4}\\.\\d{2}\\.\\d{2}-\\d{2}\\.\\d{2}\\.\\d{2}_.*$",
                         "\\d{4}\\.\\d{2}\\.\\d{2}-\\d{2}\\.\\d{2}\\.\\d{2}$",
                         "\\d{7}", "\\d{6}", "\\d{5}", "\\d{4}" ]

example strings:
IMG_1234_5678
2010.10.03 Feiertag
IMG_12345_2015.10.04-17.45.30_Test
IMG_12345_12346_12347

Answer 1

Without the expected results for your example strings I won't be able to comment on the actual algorithm. I wouldn't expect it to be correct though. There also are a lot of other issues.

The spacing between the colons and their associated variable names was inconsistent. Better to make it the same everywhere (in my example no space before the :, and one after). This also removes the alignment of the colons. This looks nice, but is not really maintainable. If you add a property with a longer name you either destroy the nice aligned look or you have to do extra work to update the spacing for all other properties. This might not seem too bad, but if multiple people work on this code this will guarantee merge conflicts.

The names of the init parameters should also match the property names. So better change pos to firstIndex as this tells more clearly what this property means.

If you feel you need to comment what a property is fore better use proper documentation comments which go before the property and start with three slashes (///). Even better would be a name like patternIndex that doesn't need an explanation and is consistent with the rest of the code.

Be consistent with var and let. In your code there was no mutation of matched, so better to use let there as well.

In this simple struct case the explicit initialiser would not be needed. Swift provides one for each property automatically. But since you usually use the value -1 for the matched property it makes sense to add an explicit initialiser with a default value for the matched parameter. Even though this is not required, I would also move parameters with a default value to the end of the parameter list.

Since you are using matched to store the index of the matching pattern or -1 if no pattern is found for this string part it makes sense to use an optional int instead. That way the compiler will force you to check wether you actually have an index before you use it.

The function toString() in the extension is unused, so this can go as well. Generally there is no reason for this function, the text property can be accessed directly instead. Since it is let this is as safe. The only reason to add such a function would be to conform to some protocol, but this is not the case here.

So the whole StringPart struct should look like this:

struct StringPart {
    let text: String
    let firstIndex: Int
    let patternIndex: Int?

    init(searchText: String, firstIndex: Int, patternIndex: Int? = nil) {
        self.text = searchText
        self.firstIndex = firstIndex
        self.patternIndex = patternIndex
    }
}

Next to your two main functions. First thing to note there is that function names should start with lowercase letters. Then you shouldn't use abbreviations (like Nums) just to type less, that's why we have autocomplete. The parameter names should be consistent. And from the callers point of view you don't need to specify the Array part in the name, this is clear from the context.

For the second function it is not clear wether the patternIndex parameter should have a default value of zero. It's always specified in this example so I would argue that it's better left out.

So better names/signatures would be like this:

func splitNumbers(searchText: String, patterns: [String]) -> [StringPart]
func splitNumbers(searchText: String, pattern: String, patternIndex: Int) -> [StringPart]

Since this works closely with the searchText string another option would be to put this code in an extension of String:

extension String {
   func splitNumbers(patterns: [String]) -> [StringPart]
   func splitNumbers(pattern: String, patternIndex: Int) -> [StringPart]
}

Inside splitNumArray the index variable is unused, so get rid of it. Instead of initialising the results array as empty and then appending the first element you can initialised it as an array with that element in the first place. The comment about that is not needed either - it doesn't say anything more than the code itself. Generally comments shouldn't explain that the code does, but why it does so.

Manually counting the index for a for-in loop is not needed, as Sequence provides a sequence of (index, value) tuples using the enumerated() function which should be used for the outer loop.

So far we can write splitNumbers like this:

func splitNumbers(searchText: String, patterns: [String]) -> [StringPart] {
    var results = [StringPart(searchText: searchText, firstIndex: 0)]

    for (patternIndex, pattern) in patterns.enumerated() {
       ...
    }

    return results
}

The inner loop is OK like this. Here we actually need to manually calculate the indices. Instead of the remove followed by the insert replaceSubrange(_:with:) with a one-element subrange would be more efficient though. The name res1Run is not very good either, run would be more appropriate. With those changes the inner loop would look like this:

  var resultIndex = 0
  for result in results {
      if result.patternIndex == nil {
          let run = splitNumbers(searchText: result.text, pattern: pattern, patternIndex: patternIndex)
          results.replaceSubrange(resultIndex...resultIndex, with: run)
          resultIndex += run.count - 1
      }
      resultIndex += 1
  }

Another option would be to replace the explicit loop with flatMap, which removes the burden of manually handling the indices:

  results = results.flatMap { (result: StringPart) -> [StringPart] in
    if result.patternIndex == nil {
      return splitNumbers(searchText: result.text, pattern: pattern, patternIndex: patternIndex)
    } else {
      return [result]
    }
  }

The flatMap version may be more "swifty", but it could be less efficient. I would use it until profiling shows that it is a performance bottleneck and the other version is actually more efficient.

Some of the same things apply to the second function. But this has some much bigger issues.

The first is an efficiency issue. You run the regex repeatedly on the string after the first match, even though the first call already returns all matches. Also the part of the string after the match is repeatedly added to the array just to be removed again.

The second one is that you can't use the string's character count in the ranges for the regex matching. The foundation string APIs are based on UTF-16 while a Swift character can consist of multiple UTF-16 code points. Try sticking strings with emoji into your function to see it produce wrong results. The pragmatic solution would be to use NSString instead of the Swift String in there.

Also the calculation of the firstIndex value seems off. It easily produces indices which are way outside of the original string. I am doing a dangerous thing here and assume that it is supposed to be the index of the StringPart in the original string. To calculate this we also need to pass the firstIndex of the part we are currently analysing to the split function and we get this:

func splitNumbers(searchText: String, pattern: String, patternIndex: Int, firstIndex: Int = 0) -> [StringPart] {

  let regex = try! NSRegularExpression(pattern: pattern, options: [])

  let s = searchText as NSString
  let matches = regex.matches(in: s as String, options: [], range: NSRange(location: 0, length: s.length))

  var results: [StringPart] = []
  var index = 0
  for match in matches.lazy.flatMap({ $0.numberOfRanges > 0 ? $0.rangeAt(0) : nil }) {
    if index < match.location {
      let beforeString = s.substring(with: NSRange(location: index, length: match.location - index))
      results.append(StringPart(searchText: beforeString, firstIndex: firstIndex + index))
    }

    if match.length > 0 {
      let foundString = s.substring(with: match)
      results.append(StringPart(searchText: foundString, firstIndex: firstIndex + match.location, patternIndex: patternIndex))
    }

    index = match.location + match.length
  }

  if index < s.length {
    let endString = s.substring(with: NSRange(location: index, length: s.length - index))
    results.append(StringPart(searchText: endString, firstIndex: firstIndex + index))
  }

  return results
}

Of course then the call needs to be adapted so it looks like this:

splitNumbers(searchText: result.text, pattern: pattern, patternIndex: patternIndex, firstIndex: result.firstIndex)