How can I refactor this code to make it more generic?

Question

I have the following template that is stored in a text file:

Hello, Thanks for Buying the {year} {make} {model}.

year, make, and model come are stored in a template configuration table and the actual values are stored in a Car table.

Currently, I get all fields that are to be used in the template (year, make, and model) and then I get the values for them from the Car table and I replace the token ({year}, {make}, {model}) with the value from the database, so it ends up looking like something:

Hello, Thanks for Buying the 2013 Honda Civic.

This is all fine when everything comes from the Car table because if the template adds needs a new field, like {color}, I just add it to the template configuration table and everything works and the template configuration table is per user, so if one user did not want {color}, that could easily be done. The problem I run into is when the data does not come from Car, but could potentially come from other tables, for example:

Hello, Thanks for Buying the {year} {make} {model}.

Your order number is {order_no} and you can contact

customer service at {customer_service_number} if you have any problems.

For the sake of this example, lets assume that {order_no} comes from an orders table and {customer_service_number} comes from a CustomerService table.

I was thinking of having a method like this:

var text = @"Hello, Thanks for Buying the {year} {make} {model}.
Your order number is {order_no} and you can contact customer service at {customer_service_number} if you have any problems.";

//This is for fields that are found directly on the Car object
foreach(var field in templateFields)
{
    var val = car.GetType().GetProperty(field).GetValue(car,null);

    text = text.Replace(field, val);
}

//Replace other fields
var order_no = order.GetOrderNo();

text = text.Replace("{order_no}",order_no);

var customer_service_number = customerService.GetCustomerServiceNumber();

text = text.Replace("{customer_service_number}",customer_service_number);

The above works fine if the everyone wants to see the extra fields, but if only several users want to see the order_no and/or the customer_service_number part, then calling one or both of them is pointless.

Also, if a customer wants to see additonal information, then I have to have another call like:

text = text.Replace("{number_of_oil_changes}",car.NumberOfOilChanges);

Then I end up with a bunch of useful and/or useless methods depending on the customer, so how can I avoid having to write all these extra methods to get information that is dependent on the customer?

Why would a customer *not* want to see all of the information? — Dan Teesdale, Oct 19 '13 at 21:12
@DanTeesdale - This is a simplified example, but let's just say that some just don't car for the extra info and some do. — xaisoft, Oct 19 '13 at 21:16
Take a look at [this](http://stackoverflow.com/questions/159017/named-string-formatting-in-c-sharp) SO answer maybe can help. — Alessandro D'Andria, Oct 19 '13 at 22:36

score 2 · Answer 1 · answered Oct 19 '13 at 22:13

I think this might be a case where you should actually go for a less generic approach, because user interfaces are usually not generic. For example you might later realize that the customer who wants to see the model without the year should see the model in a different format than the customer who sees the year. These are different screens so there might be different rules, and having code specific to each screen will make the job easier later.

I would create a flat structure tailored specifically to the template's needs. That even includes the fields that come directly from Car, because it means you will not need to update your templates if your table structure changes. Suppose you split "model" into two columns for easier filtering or something -- in that case you would only need to update your structure so that the "model" field is set to model1 + model2 instead of changing all your templates to {model1}{model2}.

To avoid loading redundant information, you could always just check the template for which fields it includes. A better way, in my opinion, would be to use a class with "lazy" properties for your template structure (properties whose values are only loaded once they are requested for the first time):

class OrderDisplay {
    private readonly Order _order;

    public OrderDisplay(Order order) {
        _order = order;
    }

    private string _orderNo;
    public string OrderNo {
        get {
            if (_orderNo == null) _orderNo = _order.GetOrderNo();
            return _orderNo;
        }
    }

    ...other properties...
}

Using classes like this, you could dynamically replace each {...} in the template by looking up a matching property on the class. And whenever a template does not include certain properties, they will naturally not be called.

How can I refactor this code to make it more generic?

1 Answers1