.Net - Strategies to avoid magic string

Question

In code at work, we have many uses of magic strings like the following code snippet:

if (user.HasRight("Profile.View")) {...}

So there are many places where we pass a string as a parameter to see if the user has a specific right. I don't like that because that generates a lot of magic strings.

What would be a better way of doing it?

Enum, Constant, class ?

Answer 1

In that specific case, use an Enum. There will be no magic strings and if the Enum changes (in a way that would break the magic strings solution), the app will no longer compile.

public enum ProfilePermissions
{
    View,
    Create,
    Edit,
    Delete
}

Then you can simply have:

if(user.HasRight(ProfilePermissions.View)) { }

You could also use a class, but then you limit yourself when it comes to more complex scenarios. For instance, a simple change of the Enumeration to something like:

public enum ProfilePermissions
{
    View = 1,
    Create = 2,
    Edit = 4,
    Delete = 8
}

Would allow you to use bitwise operators for more complex permissions (for example, a situation where a user needs either Create or Delete):

if(user.HasRight(ProfilePermissions.Create | ProfilePermissions.Delete));

Answer 2

You can do constant strings in C#.

You could define all of the strings in a header like this:

const string PROFILE_VIEW "Profile.View";

Not sure if this is the "best" way, but its certainly better than having magic values in the code.

Answer 3

This is common enough in the .NET framework as well. Examples are System.Windows.DataFormats and System.Net.WebRequestMethods.Http. You'd want the readonly variety:

public static class MumbleRights {
  public static readonly string ProfileView = "Profile.View";
  // etc..
}

Answer 4

Create a class which strongly-types those properties, like

public static class UserInfo
{
  public static bool CanViewProfile { get { return User.HasRight("Profile.View"); } }
}

This will keep your "magic strings" in one place within your code. An enum will also work, but isn't as readable in my opinion.

Note: my example is intended to act as a property proxy for the logged in user, thus the static class. If you wanted something that would work on more immediate data (say, a list of users), this type of class would need to be non-static and instantiated on per-user-account basis.

Answer 5

I second the way shown by "Justin Niessner". But in some cases I would rather prefer writing following construct of code.

public  class User
    {
        public Permission Permission { get; set; }

    }
    public abstract class Permission
    {

    }
    public class ViewPermission:Permission
    {

    }

and you can consume it as

User user=new User();
            if(user.Permission is ViewPermission)
            {

            }

Answer 6

Extension methods! Keep them in the same place to keep track of all magic strings.

public static class UserRightsExtensions {
  public static bool CanReadProfile(this User user)
  {
    return user.HasRight("Profile.View");
  }

  // etc..
}

Then you can:

if (user.CanReadProfile()) .....