1
\$\begingroup\$

The basic idea is to replace all special expressions, e.g., [%InvoiceNo%], [%DueDate%], in a string with the properties of an object, Invoice invoice.

string str = "Your invoice [%InvoiceNo%] will be due on [%DueDate%]";
Invoice invoice = new Invoice { InvoiceNo = "123456", DueDate = DateTime.Parse("2011.12.29") };
string result = GetNewValue(str, invoice);
//The expected result is: Your invoice 123456 will be due on 11/29/2011

I think the simplest way to implement GetNewValue() could be

private string GetNewValue(string str, Invoice invoice)
{
    str = str.Replace("[%InvoiceNo%]", invoice.InvoiceNo);
    str = str.Replace("[%DueDate%]", invoice.DueDate.ToString());
    return str;
}

However, in my case, Invoice has a hundred properties and the target string only contains one or two properties. I rewrite GetNewValue() in the way I think it should work more efficiently. Here is my code

private string GetNewValue(string str, Invoice invoice)
{
    List<string> expressions = new List<string>();
    List<string> fields = new List<string>();

    var startIndices = str.IndicesOf("[%");
    var endIndices = str.IndicesOf("%]");
    int startLen = "[%".Length;
    int endLen = "%]".Length;

    for (int i = 0; i < startIndices.Count; i++)
    {
        expressions.Add(str.Substring(startIndices[i], 
                                      endIndices[i] - startIndices[i] + endLen));
        fields.Add(str.Substring(startIndices[i] + startLen, 
                                 endIndices[i] - startIndices[i] - startLen));
    }

    for (int i = 0; i < expressions.Count; i++)
        str = str.Replace(expressions[i], invoice.GetValueByName(fields[i]).ToString());

    return str;
}

StringExtender IndicesOf()

public static List<int> IndicesOf(this string target, string search)
{
    List<int> indices = new List<int>();
    int startIndex = 0;
    int index;

    while ((index = target.IndexOf(search, startIndex)) > -1)
    {
        indices.Add(index);
        startIndex = index + 1;
    }

    return indices;
}

GetValueByName() using Reflection

public class Invoice
{
    // Properties
    ......

    public object GetValueByName(string name)
    {
        PropertyInfo myProperty = typeof(Invoice).GetProperty(name);
        return myProperty.GetValue(this, null);
    }
}

How can I improve this code? Fast, less memory usage, or one-liner
Don't tell me the first approach is better. Orz
Thanks :)

\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

You can do it in one loop instead of getting all the indexes separately. Just off the top of my head:

        string str = "Your invoice [%InvoiceNo%] will be due on [%DueDate%]";

        int startIndex = 0;

        while ((startIndex = str.IndexOf("[%", startIndex)) > -1)
        {
            int endIndex = str.IndexOf("%]", startIndex);
            string exp = str.Substring(startIndex, endIndex - startIndex + 2);
            string field = str.Substring(startIndex + 2, endIndex - startIndex - 2);
            str = str.Replace(exp, invoice.GetValueByName(field).ToString());
            startIndex = 0;
        }

Some points to note here:

  1. I assume you are guaranteed to see the %] if you find the [%
  2. I didn't test for edge cases like what if str = "[%invoice%]"

This is a neat solution but the more I think about your problem the more I wonder if you can engineer it so you pass what fields you want to your invoice object and it just returns you the correct string so you don't need to mess around with placeholders... food for thought :)

\$\endgroup\$
2
  • \$\begingroup\$ This solution is brilliant!! I like it :) \$\endgroup\$ Commented Nov 30, 2011 at 7:01
  • \$\begingroup\$ I ran into this thread while looking for a library to do something similar. Our requirements looked to be a little different but very similar in concept. I've created a small library that allows you template @obj.prop@ placeholders into strings and replace them with other things in a property. \$\endgroup\$
    – GetFuzzy
    Commented Jan 12, 2021 at 2:57
2
\$\begingroup\$

str.Replace does a lot of copying behind the scenes. Especially if you have a complex string, you'd be better off with a StringBuilder in a loop. Something like (untested code):

StringBuilder sb = new StringBuilder();
int idx = 0;
while (true) {
    int nextIdx = str.IndexOf("[%", idx);
    if (nextIdx < 0) break;
    // Copy unexpanded text
    sb.Append(str, idx, nextIdx - idx);
    // Find end
    int endIdx = str.IndexOf("%]", nextIdx);
    if (endIdx < 0) throw new Exception("Unmatched [%");
    // Copy the substitution and then skip past it
    string field = str.Substring(nextIdx + 2, endIdx - nextIdx + 2);
    sb.Append(GetValueByName(invoice, field));
    idx = nextIdx + 2;
}
// Copy the tail.
sb.Append(str, idx, str.Length - idx);
return sb.ToString();

NB I've assumed that GetValueByName is now a static field - it makes no sense to me for it to be a method of Invoice, as that both pushes functionality into Invoice which is nothing to do with invoicing, and limits its applicability. I would be tempted to make it an extension method of object.

Also, I've used your [% for consistency, but when I do things like this I prefer to use { and reuse string.Format's syntax. I also handle :formattingString and take an IFormatProvider as an argument.

\$\endgroup\$
3
  • \$\begingroup\$ what if the old value is: A{2,2}. I will replace it by accident :) \$\endgroup\$ Commented Dec 2, 2011 at 3:08
  • \$\begingroup\$ @WalterHuang, what do you mean by "the old value"? The format string? I could turn that around and ask what if it was [%2,2%]? You have to consider whether your use cases are going to require an escaping mechanism. \$\endgroup\$ Commented Dec 2, 2011 at 7:32
  • \$\begingroup\$ hmm ... OK, you are right :) \$\endgroup\$ Commented Dec 2, 2011 at 12:36

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