Splitting a path string

Question

I have the following C# method to split a path string. The only thing I know is a fixed string in the path and I have to split the full path into two parts, first part should be one level below the fixed string and the rest should be the second part.

For example, if I have the following path:

 string mainText = @"C:\Abc\Fixed\MyTemp\Japan\Tokyo";

then my firstPart would be "C:\Abc\Fixed\MyTemp" and second part "Japan\Tokyo"

I want to improve this method in terms of memory and speed.

private static void SplitPath(string mainText, out string firstPart, out string secondPart)
 {
 firstPart = string.Empty;
 secondPart = string.Empty;
 if (!string.IsNullOrEmpty(mainText))
 {
 string strConstatnt = "Fixed";
 List<string> splitted = mainText.Split(new char[] { '\\' }).ToList();
 int indexToFixed = splitted.IndexOf(strConstatnt);
 StringBuilder sbFirst = new StringBuilder();
 StringBuilder sbSecond = new StringBuilder();
 if (indexToFixed >= 0)
 {
 for (int i = 0; i < splitted.Count; i++)
 {
 if (i < (indexToFixed + 2))
 {
 sbFirst.Append(splitted[i] + "\\");
 }
 else
 {
 break;
 }
 }
 for (int i = (indexToFixed + 2); i < splitted.Count; i++)
 {
 sbSecond.Append(splitted[i] + "\\");
 }
 }
 if (sbFirst.Length > 0)
 {
 firstPart = sbFirst.Remove(sbFirst.Length - 1, 1).ToString();
 }
 if (sbSecond.Length > 0)
 {
 secondPart = sbSecond.Remove(sbSecond.Length - 1, 1).ToString();
 }
 }
 }

• Ben Aaronson: Is there a reason you're focussed on performance? Without knowing the wider context of how this is being used, I'd guess it's not something performance-critical.
• antred: Grammar nazi comment: "splitted" is not a word. The past tense of split is split.

Answer 1

I took the liberty to make the exercise as well to see how our approaches differ. I don't think there are many remarks in terms of performance although StringBuilder.Remove() is a red flag in that area.

All together I think my code provides a more easily readable solution which should help in terms of human-reading efficiency. In the end I pretty much abstracted the things you do under library features.

public void SplitPath(string mainText, out string firstPart, out string secondPart)
{
 const string pivot = "Fixed";
 const char delimiter = '\\';
 var entries = mainText.Split(delimiter);
 int pivotIndex = entries.Length;
 for (var i = 0; i < entries.Length; i++)
 {
 if (entries[i] == pivot)
 {
 // Take index + 2 because linq.Take/Skip is 1-based
 // Optionally take index + 1 to avoid out-of-bounds when the pivot is the last element
 pivotIndex = (i == entries.Length) ? i + 1 : i + 2;
 break;
 }
 }
 firstPart = string.Join(delimiter.ToString(), entries.Take(pivotIndex));
 secondPart = string.Join(delimiter.ToString(), entries.Skip(pivotIndex));
}

I have tested this code with inputs C:\Abc\Fixed\MyTemp\Japan\Tokyo, C:\Abc\Fixed and C:\Abc.

If you'd want to take your approach after all, a few remarks:

• Try to early return by using a negated if-statement condition.
• Put your delimiter in a variable
• Avoid magic values
• Avoid removing data from the StringBuilder

Answer 2

Using String.Substring() makes this pretty easy:

 public static void SplitPath(string mainText, out string firstPart, out string secondPart)
 {
 firstPart = mainText.Substring(0, mainText.LastIndexOf('\\'));
 firstPart = firstPart.Substring(0, firstPart.LastIndexOf('\\'));
 secondPart = mainText.Substring(firstPart.Length + 1);
 }

Input:

string mainText = @"C:\Abc\Fixed\MyTemp\Japan\Tokyo";

Output:

 firstPart = "C:\Abc\Fixed\MyTemp" 
 secondPart= "Japan\Tokyo"

• DarkDust: Instead of assigning firstPart twice I'd use a temporary variable instead. Also, if the input string does not have any backslashes at all LastIndexOf will return -1 and as a consequence Substring will throw an ArgumentOutOfRangeException. My C# is rusty, but I think OP's version just doesn't assign anything to the two output variables in this case.
• WernerCD: @DarkDust Temporary variable? The goal is make it more efficient. Reusing variables work towards that goal. If anything, maybe just string the two calls together firstPart = mainText.Substring(0, mainText.LastIndexOf('\\')).Substring(0, mainText.LastIndexOf('\\')). No duplicate assignment. No temporary variable creations.
• WernerCD: Additionally... where does this answer check for the location of fixed? Just because the example is primary four deep and secondary the next two... doesn't mean the real world example will be. What if it's C:\Abc\Xyz\Fixed\Foo\Bar\Baz? or C:\Abc\Fixed\Foo?
• DarkDust: @WernerCD: Introducing a temporary variable will not change the number of objects allocated, it's just one additional reference which quickly goes out of scope. I personally think it's more important to be able to read/maintain code easily than trying than trying to do premature optimizations. And your contraction is wrong: your second call to Substring will not modify the string returned from the first Substring any more as your cutting it to the exact same length again. But then again, you're absolutely right about the missing "Fixed" so the whole code is wrong anyway.

Answer 3

You can use regular expressions:

static void Main(string[] args)
{
 var input = @"C:\Abc\Fixed\MyTemp\Japan\Tokyo";
 foreach (var item in Regex.Split(input, @"(?<=Fixed\\(?=[^\\]+))"))
 Console.WriteLine(item);
}