2

I had a simple static method which would take a value and a unit type (pounds, gallons, dollars) and would just format a string. A simplified example:

public static string Format(decimal value, UnitType unit)
{
    ...
    if(unit == UnitType.Dollars)
    {
       return "$" + value;
    }
    ...
}

Now, the requirements changed, as they do, and users can select different currencies. Information about the selected currency comes from UserSettingsService. I guess I could try to refactor this static method into a instance method in some "FormattingService" and inject it as dependency everywhere I need but the issue is that it is not used just in other services, it is used in simple model classes too. I want to avoid having to go through all of the code base and make sure that this is only "FormattingService" would only get called from other service.

One solution to this problem that I have thought of is to use service locator (anti?)pattern. So, after changes, Format method might look like this:

public static string Format(decimal value, UnitType unit)
{
    ...
    if(unit == UnitType.Currency)
    {
       var service = ServiceLocator.GetService<IUserSettingsService>();
       return service.CurrencySymbol + value; // Simplified
    }
    ...
}

Is this a good solution for this case? Is there something you would do differently?

1 Answer 1

3

Ideally, your model classes should be unaware of things like user settings, or locale settings. These are responsibilities better handled in a different layer, for example, the UI layer. If you can avoid the runtime dependency, that will keep the model classes way more reusable and testable.

For example, think of the next requirement incoming where the currency is not determined by those user settings, but by from a different data pool. Maybe it is required for some report where different currencies occur side-by-side on the same page.

This leads to the question why your model classes contain calls to a Format function which can potentially depend on such things like user settings. Of course, you wrote, it initially did not, and you got changes to the requirements afterwards. But "formatting numbers and values" is always a concern which is prone to get such kind of dependencies sooner or later. So this is probably the root cause of your problem.

First thing I would check here if value formatting, especially user-settings dependend formatting, can be done in a completely different layer. It often feels convenient to put reusable formatting functions directly into the model, but in case the only place they are required are the UI layer, for UI purposes, it is probably better to introduce some FormattingHelper class in that UI layer and put the formatting there.

If that is not possible, next thing I would try is if it is sensible to change the signature of that Format function so all required information is passed by the caller. Maybe it is possible to replace the enum UnitType by a class which carries not only the unit type information, but additional information, like the currency symbol. This may lead to some cascading changes to the callers as well, since you have to pass that new information downstream from the layer where the user settings are available through the intermediate layers to the model classes.

Indeed, this can require some refactoring effort, but to my experience - in case you have the full code base under your control - this is usually worth the effort, since implicit runtime dependencies are really the road to a totally unmaintainable big-ball-of-mud architecture. For changing a signature of such a Format function in a lower layer usually the compiler can tell you where you have to add further changes, so the risk of breaking anything is not too large, if you be careful.

Your Answer

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

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