1

I have a method that takes in a dictionary and enum value and produces a string output. Enum value defines what template string to insert the dictionary key value pairs into.

public string InsertVariables(MessageType type, Dictionary<string, string> variables) { }

It works fine, but every time I use it, I find that it's hard to know what I should add to variables to make sure all tokens in the template are replaced and I have to glance at the templates every time.

What I was thinking would work as solution in this case is enforcing proper values added to variables during compilation time like so:

public abstract class MessageVariables { }

public class NewUserMessageVariables : MessageVariables {
    // different values go here parameters go here instantiated in the constructor
}

public string InsertVariables(MessageType type, MessageVariables variables) { 
    // converts variables object into dictionary for future insertion into the template.
}

This seems to enforce having all parameters required in the messages.

I have about 16 messages with possibility to having a few more later.

QUESTION: Is this an overkill and I should stick to using dictionary?

2
  • Is the constructor input data in the same format?
    – Paul W
    Jun 26, 2015 at 1:24
  • If you are asking about the string that get's used, than no, every message is different with hardly any similar variables.
    – Alexus
    Jun 26, 2015 at 16:35

2 Answers 2

1

Your concerns are valid. You're using a strongly typed language, yet venturing into a weakly-typed or untyped area. It happens, but this kind of error/constraint checking you're asking about (if nicely addressed) will increase maintainability by returning you to the benefits of being more strongly typed.

What I would probably do is analyze the template to see which variables it mentions. So you can have a notion of a regular template, and also, a counterpart notion of an analyzed template that additionally has a list of required variables that need to be bound in order to instantiate. Such analysis might plausibly be automated, or manually done with adornments, for example if you add type information not present in the template.

On the other hand, annotating the template (text) itself with type information might be a good approach, as it would allow automation in developing the analyzed template counterpart that has the variables listed.

Then you can have a notion of instantiation of the analyzed template, which can demand (or fail on lack of) the right variables. (You can also check, if desired, whether some variables being supplied are not even used.)

I'm not sure from you question whether your concern goes more to the type of the various variables rather than making sure they are all supplied (say, more or less untyped).

In essence, I'd like to change the enum into a class (maybe a base class and set of classes, one for each template even) that is/are more descriptive of what is required regarding variables.

If you go far enough with changing the enum into classes, you might just find passing class(template)-specific (constructor) parameters instead of a dictionary to be workable.

As you're in C# (and presumably Visual Studio), you might use a T4 template to generate the classes of interest from some prototype .tt file, depending on how you like to author your templates. This can work really nicely to provide proper OOP classes and types that you want from relatively easily edited data definitions.

1

It depends.

If there is some code above this that effectively enforces that the right variables are sent in (some form for example), then this is unlikely to break frequently, so making each message its own type seems like overkill.

If there is not some code above this and you're relying on your programmer's goodwill to make sure that stuff is well formed... That is probably a bad bet, and making a firmer interface will serve you well.

I would not have some MessageVariables base type unless it provides some benefit. And I would personally err heavily towards real classes/functions for each message. It makes things more discoverable, and at only 16 or so messages is not prohibitively maintainable.

2
  • That's the problem, it's solely "good will" here. That's why I feel bad about having this code in there ;) The reason for base class is to be able to have single InsertVariable method treat all of them the same. All that method does is String.Replace for each variable. I already have wrapper functions like you mentioned for each message, InsertVariables is however doing the same thing for each wrapper, hence it's single for all messages.
    – Alexus
    Jun 26, 2015 at 0:13
  • Also, I just learned that structs are not inheritable, so changed to classes.
    – Alexus
    Jun 26, 2015 at 0:15

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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