Building a summary string from a flags enum

Question

I'm trying to replicate the attributes text that appears in File Explorer, for files and directories.

I can't just use the enum names, as the summary text uses a single character for each set flag, and that character isn't always the first character of the enum member. I'm also not using the reserved FileAttribute.Device attribute. So, I'm using a string.join operation to concatenate the flag characters.

    public string AttributeSummary
    {
        get
        {
            FileAttributes fatt = (FileAttributes)_attribs;

            return string.Join(string.Empty,
                (fatt.HasFlag(FileAttributes.ReadOnly) ?          "R" : string.Empty),
                (fatt.HasFlag(FileAttributes.Hidden) ?            "H" : string.Empty),
                (fatt.HasFlag(FileAttributes.System) ?            "S" : string.Empty),
                (fatt.HasFlag(FileAttributes.Directory) ?         "D" : string.Empty),
                (fatt.HasFlag(FileAttributes.Archive) ?           "A" : string.Empty),
                (fatt.HasFlag(FileAttributes.Normal) ?            "N" : string.Empty),
                (fatt.HasFlag(FileAttributes.Temporary) ?         "T" : string.Empty),
                (fatt.HasFlag(FileAttributes.SparseFile) ?        "P" : string.Empty),
                (fatt.HasFlag(FileAttributes.ReparsePoint) ?      "L" : string.Empty),
                (fatt.HasFlag(FileAttributes.Compressed) ?        "C" : string.Empty),
                (fatt.HasFlag(FileAttributes.Offline) ?           "O" : string.Empty),
                (fatt.HasFlag(FileAttributes.NotContentIndexed) ? "I" : string.Empty),
                (fatt.HasFlag(FileAttributes.Encrypted) ?         "E" : string.Empty),
                (fatt.HasFlag(FileAttributes.IntegrityStream) ?   "V" : string.Empty),
                (fatt.HasFlag(FileAttributes.NoScrubData) ?       "X" : string.Empty));
        }
    }

Am I going about this in the most efficient way?

Answer 1

You should start by separating concerns. Your current AttributeSummary does two things:

• it maps flags to abbreviations
• it creates the string

Solution 1 - yield return

So how do we fix it? We create an extension method that will only give us the abbreviations for active flags:

public static IEnumerable<string> ToAbbreviations(this FileAttributes fileAttributes)
{
    if (fileAttributes.HasFlag(FileAttributes.ReadOnly)) { yield return "R"; }
    if (fileAttributes.HasFlag(FileAttributes.Hidden)) { yield return "H"; }
    if (fileAttributes.HasFlag(FileAttributes.System)) { yield return "S"; }
    // ...
}

and we then are able to turn the property into one-liner:

public string AttributeSummary => string.Join(string.Empty, ((FileAttributes)_attribs).ToAbbreviations());      

By separating them you can tune the ToAbbreviations however you like and the AttributeSummary stays unaffected - this is exactly what we want. If you decide to use the optimized HasFlag from @svick you just need to change the extension... which by the way is now much easier to test because you can verify that it returns the correct value for each flag before creating the string.

This is still not the most optimal solution becasue the abbreviations are dependant of the active flag so you cannot just get them all at once. There is one more way...

Solution 2 - dictionary

I find however a dictionary solution is nice one.

What you need to do is to create a mapping dictionary for the flags:

private static readonly IReadOnlyDictionary<FileAttributes, string> FileAttributesAbbreviations = new Dictionary<FileAttributes, string>
{
    [FileAttributes.ReadOnly] = "R",
    [FileAttributes.Hidden] = "H",
    [FileAttributes.System] = "S",
};

Modify the ToAbbreviations extension to use the dictionary:

public static IEnumerable<string> ToAbbreviations(this FileAttributes fileAttributes)
{
    return fileAttributes.Explode().Select(x => FileAttributesAbbreviations[x]);        
}

And write one more extension to explode the flags:

public static IEnumerable<FileAttributes> Explode(this FileAttributes fileAttributes)
{
    return Enum.GetValues(typeof(FileAttributes))
        .Cast<FileAttributes>()
        .Select(x => fileAttributes & x)
        .Where(x => x != 0);
}

The AttributeSummary property does not change but the flags and abbreviation mapping is even easier to understand and to maintain now.

I prefer the dictionary because the is the most reusable solution. If you need those abbreviation in other places you don't have to repeat yourself and can use the dictionary in several places.

Answer 2

Am I going about this in the most efficient way?

If you're worried about efficiency, then Enum.HasFlag is known to be fairly slow.

Instead, I would use a very simple helper method:

private static bool HasFlag(FileAttributes value, FileAttributes flag)
    => value & flag != 0;

Though if you're doing that, you might as well expand the helper function to give you the flag character:

private static string GetAttributeCharacter(
    FileAttributes value, FileAttributes flag, string character)
    => (value & flag != 0) ? character : string.Empty;

---

I'm on the fence whether the helper method should access _attribs directly or whether it should get its value in a parameter. On one hand, reading the field over and over is likely going to be slower. On the other hand, that's the kind of microoptimization that does not matter in the vast majority of cases and accessing it directly would remove quite a lot of duplicated code.

---

Using string.Concat(…) instead of string.Join(string.Empty, …) is simpler and should be faster.

---

If you're on C# 6.0, you can avoid some code repetition by using using static System.IO.FileAttributes;.

---

If it's clear what type a variable is, like your fatt, most code styles accept using var for its type. Also, I think that abbreviations like fatt are generally not worth it: they save you some type, but make understanding the code later harder.

Also, I don't understand why is the cast to FileAttributes necessary, but I'm going to assume there is a good reason for it.

---

Put together:

private string GetAttributeCharacter(FileAttributes flag, string character)
    => ((FileAttributes)_attribs & flag != 0) ? character : string.Empty;

public string AttributeSummary
{
    get
    {
        return string.Concat(
            GetAttributeCharacter(ReadOnly,          "R"),
            GetAttributeCharacter(Hidden,            "H"),
            GetAttributeCharacter(System,            "S"),
            GetAttributeCharacter(Directory,         "D"),
            GetAttributeCharacter(Archive,           "A"),
            GetAttributeCharacter(Normal,            "N"),
            GetAttributeCharacter(Temporary,         "T"),
            GetAttributeCharacter(SparseFile,        "P"),
            GetAttributeCharacter(ReparsePoint,      "L"),
            GetAttributeCharacter(Compressed,        "C"),
            GetAttributeCharacter(Offline,           "O"),
            GetAttributeCharacter(NotContentIndexed, "I"),
            GetAttributeCharacter(Encrypted,         "E"),
            GetAttributeCharacter(IntegrityStream,   "V"),
            GetAttributeCharacter(NoScrubData,       "X"));
    }
}