Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Design to prevent magic numbers and strings

I am building a service that retrieves from and submits to a third party. The third party delivers a complex data model which includes three different status types, all of them integers with an obscure number series. An example is provided below (key:value):

HandlerStatus          CustomerStatus   ArchiveStatus
---                    ---              ---
activeUnallocated:40   draftOpen:0      openOpen:0      
activeAllocated:26     submitted:2      closed:1
activePromoted:102     approved:100
Declined:2             declined:1
...                    ...

There are only a limited amount of combinations, and each of the combinations have a term (submitted, delegated to another user, waiting for confirmation, etc). For example:

Combination called "Submitted":
HandlerStatus has to be "activeUnallocated" (40)
CustomerStatus has to be "submitted" (2)
ArchiveStatus has to be "openOpen" (0)

We can expect additional values to be added, and the user friendly names might change over time.
In addition, there are some actions like submitting and delegating to other users that don't require users to choose new values, which means that the service needs to know of a set of combinations that have to be set when these actions occur.

  • I have to be able to a define user friendly display name for each status value in each status type
  • I have to be able to receive a new status value from a user and map that the correct status type
  • I have to be able to look up the values of a defined combination
  • I have to be able to look up a defined combination based on a set of status values


I've thought of three different solutions with varying pros and cons.

1. Defining en enum for each status type

Pros:

  • Strongly typed
  • Maintenance limited to one file

Cons:

  • Enums
  • String formatting for UI presentation


2. Defining the key/value pairs in an external JSON file

Pros:

  • No code changes required for additions
  • Can be updated during runtime

Cons:

  • Requires magic strings/numbers for actions that have associated combinations (actions without user input)
  • Frequent reading from the file system


3. Create proxy classes for the different available status combinations

Pros:

  • Strongly typed
  • Flexible

Cons:

  • Requires code maintenance when changes occur in the third party system


I've tried researching best practices and patterns to see which solution is best suited, both for maintainability and according to good code design, but I'm not getting anywhere (in the meantime I'm relying heavily on magic strings and numbers during development).

like image 685
Marius Agur Avatar asked Dec 26 '15 12:12

Marius Agur


People also ask

How do you suppress magic numbers?

You can of course disable the rule altogether or add @Suppress("MagicNumber") to the expression.

Why would you avoid magic numbers in code?

You would avoid magic numbers cause other people viewing your code might not understand why you're doing what you're doing... e.g. const myNum = 22; const number = myNum / 11; right now my 11 could be people or bottles of beer or something so instead I would change 11 to a constant such as inhabitants.

How do you avoid magic numbers in C++?

Programming isn't magic, so don't incant it: Avoid spelling literal constants like 42 or 3.14159 in code. They are not self-explanatory and complicate maintenance by adding a hard-to-detect form of duplication. Use symbolic names and expressions instead, such as width * aspectRatio .

Are strings magic numbers?

For those who are unfamiliar with the terms, a magic number is any numeric literal used in your program other than the numbers 0 and 1, and a string literal is any quoted string.


1 Answers

First and foremost: You are correct in avoiding magic numbers and strings.

A rule of thumb is that is its an integer that doesn't change (often) then enums are good. If its a string that doesn't change often then a static (shared) class with strings in it is good. Your goal in these cases is to centralize the definition. This gives you several advantages such as avoiding typos (in a string), making the code more readable (with enum names instead of numbers) and you can trace uses (find usages). Note that you may not want to use the const keyword unless your values are set in stone and will never change.

For values that change often you want to consider program logic to handle these. Nobody hardcodes customer names and adresses (hopefully), so if your statuses changes often you want to handle them as you would any other data. If they are tied to the program logic then you need to define what program logic is required and somehow link up the statuses.

For values that will change less frequently you have the choice between hardcoding (enum/string) and reading from config file. Hardcoding obviously requires recompiling and then you should consider the long-term cost. What takes you 5 minutes to do today may take days in the future. 2 years from now another developer assigned the task of updating one of these values may spend days setting up the right development environment, compiling and testing. Seeing how easy it is to just read the values from a file my recommendation would usually be to use config files.

For your particular problem it seems what you need is a translation table. Your application has a specific task to perform. The answer would seem to make the application generic and have the translation in a file. You want to keep both constraints on status updates (what will transform what statuses into what) and friendly displays (for users) in a file (or sql-table). In short making the application agnostic about what transitions the status fields go through.

Do not worry about file IO. Its easy to implement caching.

private static TranslationObject _translationObject = null;
public static TranslationObject GetTranslationObject() {
    if (_translationObject == null)
       lock (_translationObject)
           _translationObject = JsonConvert.DeserializeObject<TranslationObject>(File.ReadAllTextt("TranslationTable.json"));
    return _translationObject;
}

Even in web apps the app will stay alive for some time so this would cache the file between requests. And anyway the OS should have enough memory to cache the disk IO for a few kilobytes of data.

To illustrate one of many ways to implement I've added a sample where a struct is used in combination with Dictionary for fast two-way lookup between a friendly status and a combination of other statuses. I'm not sure if this is exactly what you are asking for, but hopefully it can be of some help.

void Main()
{
    // Get translation
    var translationObject = GetTranslationObject();
    // Find friendly status based on combo
    var friendly1 = translationObject.ComboStatusToFriendlyStatus[new StatusCombo(0, 30, 5)];
    // Find combo based on friendly status
    var combo1 = translationObject.FriendlyStatusToComboStatus[0];
}

public struct StatusCombo
{
    // Please note that fields are readonly for immutability.
    // This is particularly important since the GetHashCode() value is used in dictionaries.

    // Note that status fields can also be strings (because we use .GetHashCode() in GetHashCode()).
    public readonly int Status1;
    public readonly int Status2;
    public readonly int Status3;
    [JsonConstructor]
    public StatusCombo(int status1, int status2, int status3)
    {
        Status1 = status1;
        Status2 = status2;
        Status3 = status3;
    }

    public override int GetHashCode()
    {
        unchecked
        {
            int hashCode = Status1.GetHashCode();
            hashCode = (hashCode * 397) ^ Status2.GetHashCode();
            hashCode = (hashCode * 397) ^ Status3.GetHashCode();
            // ... Repeat for every extra statuscode you add
            return hashCode;
        }
    }
}

public class TranslationObject
{
    public Dictionary<int, string> Status1Mapping;
    public Dictionary<int, string> Status2Mapping;
    public Dictionary<int, string> Status3Mapping;
    public Dictionary<int, string> FriendlyStatus;

    public Dictionary<int, StatusCombo> FriendlyStatusToComboStatus;
    [JsonIgnore]
    public Dictionary<StatusCombo, int> ComboStatusToFriendlyStatus;
}

private static TranslationObject _translationObject = null;
public static TranslationObject GetTranslationObject()
{
    if (_translationObject == null)
        lock ("Reading _translationObject")
        {
            _translationObject = JsonConvert.DeserializeObject<TranslationObject>(File.ReadAllText(@"TranslationTables.json"));
            // Populate reverse lookup
            _translationObject.ComboStatusToFriendlyStatus=new Dictionary<UserQuery.StatusCombo, int>();
            foreach (var t in _translationObject.FriendlyStatusToComboStatus)
                _translationObject.ComboStatusToFriendlyStatus.Add(t.Value, t.Key);

        }
    return _translationObject;
}

A sample JSON file:

{
  "Status1Mapping": {
    "0": "Status1_0",
    "10": "Status1_1"
  },
  "Status2Mapping": {
    "30": "Status2_0",
    "55": "Status2_1"
  },
  "Status3Mapping": {
    "5": "Status3_0",
    "2": "Status3_1"
  },
  "FriendlyStatus": {
    "0": "Submitted",
    "1": "Received"
  },
  "FriendlyStatusToComboStatus": {
    "0": {
      "Status1": 10,
      "Status2": 55,
      "Status3": 2
    },
    "1": {
      "Status1": 0,
      "Status2": 30,
      "Status3": 5
    }
  }
}

And the code I used to populate the sample JSON:

var tro = new TranslationObject();
tro.Status1Mapping = new Dictionary<int, string>();
tro.Status2Mapping = new Dictionary<int, string>();
tro.Status3Mapping = new Dictionary<int, string>();

tro.Status1Mapping.Add(0, "Status1_0");
tro.Status1Mapping.Add(10, "Status1_1");
tro.Status2Mapping.Add(30, "Status2_0");
tro.Status2Mapping.Add(55, "Status2_1");
tro.Status3Mapping.Add(5, "Status3_0");
tro.Status3Mapping.Add(2, "Status3_1");

tro.FriendlyStatus = new Dictionary<int, string>();
tro.FriendlyStatus.Add(0, "Submitted");
tro.FriendlyStatus.Add(1, "Received");

tro.FriendlyStatusToComboStatus = new Dictionary<int, UserQuery.StatusCombo>();
tro.FriendlyStatusToComboStatus.Add(0, new StatusCombo(10, 55, 2));
tro.FriendlyStatusToComboStatus.Add(1, new StatusCombo(0, 30, 5));

File.WriteAllText(@"TranslationTables.json", JsonConvert.SerializeObject(tro));
like image 162
Tedd Hansen Avatar answered Sep 30 '22 15:09

Tedd Hansen