.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 ?

Wingless

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));

Personification

Depending on the requirements this can also be turned into a flags-enum, i.e. where values can be combined using |, for example by using the values 0x1, 0x2, 0x4, etc... – Cryptonymous

Good answer and my answer was identical. I might add that the point of an enum is to create an "enumerated list" of constants. Grouping constants where they can be listed like this is a great idea and gives the program a more logical flow as well. – Hajji

In case if also a string representation is required, e.g. to store the permission set in a human-readable way, you can either simply call ProfilePermissions.View.ToString() to get the string "View", or if you need a custom string representation, you could add an adorning Description attribute as described here: blogs.msdn.com/b/abhinaba/archive/2005/10/20/483000.aspx – Cryptonymous

What happens when you need to add permissions? Recompile? Constants and enums should be used for static data. View, create, edit and delete are fine, but I would not call the enum ProfilePermissions, just Permissions. In the profile class add a property for permissions and you are done. The entire thing can be managed in an outer data source and your code keeps it's readability and security. – Fuscous

I'm a big fan of enums replacing magic strings. One other nice feature is where you absolutely have to pass in a string (say because it's a third party library that only handles strings) you can use the ToString() method to keep the strength of using enums, but still have a string. It only works for alphabetical characters but is handy. e.g. <code> ThirdPartyLibrary.GetResponse(string role) </code> could take <code> enum Roles { Admin, Editor, Guest } </code> as <code> ThirdPartyLibrary.GetResponse(Roles.Guest.ToString()) </code> – Mars

Answer 2

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..
}

Normie

@Markus, if public constants are used from an external assembly, they are baked in; if you later change your public constant, the external assembly will still use the constant it saw when it was compiled. – Amparoampelopsis

#277510 – Brink

Right, there are few constants in the world that are invariant enough to allow them to be public constants. Math.Pi is okay. – Normie

Answer 3

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()) .....

Selfsatisfaction

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.

Aboard

CanViewProfile makes no sense as a static property in my opinion and should rather be an instance method on a user object. The snippet suggests that there would be a static class for each user? – Cryptonymous

No, I'm not suggesting a static class for each user. The class above just acts as a proxy to properties of the logged-in user. (I used "Alice" because I'm used to using alice, bob, charlie, david, etc instead of foo/bar. – Aboard

Answer 5

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.

Monocot

Answer 6

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)
            {

            }

Leek