How can I refactor this C# code using Split()?

Question

How can I refactor this so that numberOfItems doesn't have to be declared as a variable?

//method: gets the text in a string in front of a marker, if marker is not there, then return empty string
//example: GetTextAfterMarker("documents/jan/letter043.doc","/") returns "letter043.doc"
//example: GetTextAfterMarker("letter043.doc","/") returns ""
//example: GetTextAfterMarker("letter043.doc",".") returns "doc"
public static string GetTextAfterMarker(string line, string marker)
{
    int numberOfItems = line.Split(new string[] { marker }, StringSplitOptions.None).Count();

    string result = line.Split(new string[] { marker }, StringSplitOptions.None)[numberOfItems-1];
    return line.Equals(result) ? string.Empty : result;
}

Answer 1

If you're using 3.5 this is trivial with Linq:

public static string GetTextAfterMarker2(string line, string marker)
{
  string result = line.Split(new string[] { marker }, StringSplitOptions.None).Last();
  return line.Equals(result) ? string.Empty : result;
}

Answer 2

public static string GetTextAfterMarker(string line, string marker)
{
    string result = line.Split(new string[] { marker }, StringSplitOptions.None)[line.Split(new string[] { marker }, StringSplitOptions.None).Count()-1];
    return line.Equals(result) ? string.Empty : result;
}

Why you'd want to do that though is beyond me... It's messy, to say the least.

Perhaps:

public static string GetTextAfterMarker(string line, string marker)
{
    string[] tmp = line.Split(new string[] { marker }, StringSplitOptions.None);
    string result = tmp[tmp.Length-1];
    return line.Equals(result) ? string.Empty : result;
}

Or you might even consider using .IndexOf and .Substring instead, something like this (untested):

public static string GetTextAfterMarker(string line, string marker)
{
    int index = line.LastIndexOf(marker);
    return index < 0 ? string.Empty : line.Substring(index + marker.Length);
}

Answer 3

I'd do it this way

    public static string GetTextAfterMarker(string line, string marker)
    {
        int pos = line.LastIndexOf(marker);
        if (pos < 0)
            return string.Empty;
        else
            return line.Substring(pos + marker.Length);
    }

No need to call split, no need to create an array of items, which basically you're just going to throw away.

Much faster and less resource heavy.

Answer 4

If I'm reading your code correctly, you're trying to get the last instance?

string[] splits = line.Split(new string[] { marker }, StringSplitOptions.None);
return (splits.Length == 1) ? string.Empty : splits[splits.Length-1];

Answer 5

What about:

public static string GetTextAfterMarker(string line, string marker)
{
    var items = line.Split(new[] {marker}, StringSplitOptions.None);

    string result = items[items.Length-1];
    return line.Equals(result) ? string.Empty : result;
}

Answer 6

You could always use regular expressions instead of split:

    public static string GetTextAfterMarker(string line, string marker)
    {
        if (String.IsNullOrEmpty(line))
            throw new ArgumentException("line is null or empty.", "line");
        if (String.IsNullOrEmpty(marker))
            throw new ArgumentException("marker is null or empty.", "marker");
        string EscapedMarker = Regex.Escape(marker);
        return Regex.Match(line, EscapedMarker + "([^" + EscapedMarker + "]+)").Groups[1].Value;
    }

Answer 7

var    index = line.LastIndexOf (marker) ;
return index < 0 ? string.Empty : line.Substring (index + marker.Length) ;

Answer 8

Altough I don't know wether you asked this out of simple curiosity, I would recommend you to not being afraid of creating auxiliary variables if that makes your code more readable. You will rarely degrade your code performance for using these extra variables.