Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I am trying to implement strings tokenizer in Excel via UDF using C# and Excel-Dna. Personally I found the mIRC's function $gettok pretty useful due to the lack of string functions in Excel.

Examples

getTok("Aquel que no espera vencer ya esta vencido",   1, 32) -> "Aquel"
getTok("Aquel que no espera vencer ya esta vencido",  -1, 32) -> "vencido"
getTok("Aquel que no espera vencer ya esta vencido", 2-4, 32) -> "que no espera"
getTok("Aquel que no espera vencer ya esta vencido",   0, 32) -> "Aquel que no espera vencer ya esta vencido"

Code

[ExcelFunction(Category="String Utilities", Name = "getTok")]
public static string getTok(string text, string token, int delimiter)
{
    int from = 0;
    int to = 0;

    string tokenPatter = @"(\d+)(-)(\d+)?";
    string[] tokens = text.Split(new char[] { (char)delimiter });

    Regex tokenRegex = new Regex(tokenPatter, RegexOptions.IgnoreCase);
    Match tokenMatch = tokenRegex.Match(token);

    if (tokenMatch.Success)
    {
        StringBuilder sb = new StringBuilder();
        from = short.Parse(tokenMatch.Groups[0].Value);

        if (tokenMatch.Groups.Count == 1) to = from;
        else if (tokenMatch.Groups.Count == 2) to = (short)tokens.Length;
        else to = short.Parse(tokenMatch.Groups[2].Value);

        for (int i = from; i <= to; i++)
        {
            sb.Append(tokens[i - 1] + ((char)delimiter));
        }

        return sb.ToString();
    }

    int index = int.Parse(token);
    if (index > 0) return tokens[index - 1];
    else return tokens[tokens.Length + index];    
}

Questions

  • What would you improve from above code?
  • Have you tried something similar with Excel?
share|improve this question
2  
I think you have a typo in your examples; shouldn't geTok be getTok? –  Mat's Mug Sep 5 '13 at 23:49
    
@retailcoder: Oops! typo corrected. –  Eder Sep 6 '13 at 1:17
add comment

1 Answer

up vote 5 down vote accepted

in this statement

for (int i = from; i <= to; i++)
    {
        sb.Append(tokens[i - 1] + ((char)delimiter));
    }

The way that you have it written, I don't think that i will ever hit to

also,

I would change some Variable names to make it more readable. from and to got confusing, I had to keep telling myself that those are variables. I recommend doing something like

int intFrom;
int intTo;

at the very least. so that you automatically know that those are integer variables.

also you have an Integer for a delimiter? why not just pass a char as a delimiter?

other than that I think the code looks good.

share|improve this answer
    
+1 for renaming the variables; I think from cause issues if you're using System.Linq? –  Mat's Mug Sep 5 '13 at 23:52
1  
@retailcoder I believe a variable called from won't cause any issues, unless you try to use it in an LINQ expression. –  svick Sep 6 '13 at 1:17
4  
I don't think prefixing from and to with the types make very good variable names. If you're going to be more verbose, you might as well use names like fromIndex and toIndex. –  icktoofay Sep 6 '13 at 2:38
    
it was more of an at least prefix the variables type of thing, not this is the best practice type of thing. –  Malachi Sep 6 '13 at 13:53
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.