7
\$\begingroup\$

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 on?
  • Have you tried something similar with Excel?
\$\endgroup\$
6
\$\begingroup\$

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 int for a delimiter? Why not just pass a char as a delimiter?

Other than that I think the code looks good.

\$\endgroup\$
  • 1
    \$\begingroup\$ +1 for renaming the variables; I think from cause issues if you're using System.Linq? \$\endgroup\$ – Mathieu Guindon Sep 5 '13 at 23:52
  • 1
    \$\begingroup\$ @retailcoder I believe a variable called from won't cause any issues, unless you try to use it in an LINQ expression. \$\endgroup\$ – svick Sep 6 '13 at 1:17
  • 6
    \$\begingroup\$ 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. \$\endgroup\$ – icktoofay Sep 6 '13 at 2:38
  • \$\begingroup\$ it was more of an at least prefix the variables type of thing, not this is the best practice type of thing. \$\endgroup\$ – Malachi Sep 6 '13 at 13:53
2
\$\begingroup\$

First off, getTok should be GetToken or Tokens. Don't overly abbreviate things, this goes doubly for any public API. However, even the verbose forms of those aren't good names. This function doesn't really tokenize things, or return tokens.

From the point of view of an Excel user, it seems to just be an odd way of getting a substring and it's not obvious how I'm supposed to call your function or what it should return to me.

I recommend putting more thought into your public API and utilizing the [ExcelArgument] attribute to better document it.

As a quick aside, I found this nifty looking tool that generates actual documents from ExcelDna attributes. You may find it of interest.

http://mndrake.github.io/ExcelDnaDoc/index.html

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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