Is there a better way?

Question

Is There a better way to do this.
FileInfo f = new FileInfo("C://notebook.txt");

public bool Archived
        {
            get
            {
                return (((File.GetAttributes(f.FullName)) & FileAttributes.Archive) == FileAttributes.Archive);
            }
            set
            {
                if (value == true)
                {
                    if (!this.Archived)
                    {
                        File.SetAttributes(f.FullName, File.GetAttributes(f.FullName) | FileAttributes.Archive);
                    }
                }
                else if (value == false)
                {
                    if (this.Archived)
                    {
                        File.SetAttributes(f.FullName, File.GetAttributes(f.FullName) & ~FileAttributes.Archive);
                    }
                }
            }

        }

`

Answer 1

No, that's pretty standard when dealing with flagged (bitwise rather) values.

You might wanna lose the else if bit, as booleans generally only have 2 states.

Answer 2

Well, you can always start by simplifying the way you handle the value in the setter. Then you could avoid a double-get by reading the attributes at the top of the setter.

    public bool Archived
        {
            get
            {
                return (((File.GetAttributes(f.FullName)) & FileAttributes.Archive) != 0);
            }
            set
            {
                var attributes = File.GetAttributes(f.FullName);
                bool archived = ((attributes & FileAttributes.Archive) != 0);

                if (value)
                {
                    if (!archived)
                        File.SetAttributes(f.FullName, attributes | FileAttributes.Archive);
                }
                else
                {
                    if (archived)
                        File.SetAttributes(f.FullName, attributes & ~FileAttributes.Archive);
                }
            }
        }

Now, Guffa has a point about the attributes being cached by FileInfo, though I see this more as an argument against using FileInfo in the first place. I'd prefer to store just the pathname as a string.

I also changed the bit test to compare to zero, which I should have done in the first place. Thanks, KeithS and Guffa.

And, to keep it all in one place, if we were using C# 4.0, we could say:

bool archived = attributes.HasFlag(FileAttributes.Archive);

Answer 3

In the getter, if the bitwise AND evaluates to any nonzero, the bit's set, so your getter can be shortened somewhat:

get
{
   return (((File.GetAttributes(f.FullName)) & FileAttributes.Archive) != 0);
}

Everything else is pretty much optimal; you can lose some parenthesis and one evaluation by getting rid of the braces around the if and else of the setter, and making the "else if" just an "else". Or, combine the inner and outer expressions; really, the inner expressions are not necessary as 1 | 1 == 1 and 1 & ~1 == 0, so setting it to the same value multiple times won't hurt anything.

Answer 4

Yes, as you have a FileInfo object you can use it's Attributes property instead of using the File.GetAttributes and File.SetAttributes methods:

public bool Archived {
  get {
    return (f.Attributes & FileAttributes.Archive) != 0;
  }
  set {
    if (value) {
      if (!this.Archived) {
        f.Attributes |= FileAttributes.Archive;
      }
    } else {
      if (this.Archived) {
        f.Attributes &= ~FileAttributes.Archive;
      }
    }
  }
}