3

I know there are many ways to accomplish what I am trying to do here, but I wanted to try it with LINQ, and am seeing some odd (to me) results. Hoping someone here has some wisdom to offer me.

Goal is to find the drive on the local machine where we execute with the most available free space that has been formatted to NTFS file system, excluding the %SystemDrive% volume if possible. Another caveat is that drives that are assigned a volume letter, but have not yet been formatted must be excluded from the candidate pool. This is an edge case, but one customer has already encountered it.

So, I am trying:

string systemDrive = Environment.GetEnvironmentVariable("SystemDrive") + "\\";

List<DriveInfo> localDrives = DriveInfo.GetDrives().ToList();

var bestAPIDataDrives = from localDrive in localDrives
            orderby localDrive.AvailableFreeSpace descending
            where   localDrive.DriveType == DriveType.Fixed &&
                    localDrive.DriveFormat == "NTFS" &&
                    localDrive.RootDirectory.FullName != systemDrive &&
                    localDrive.IsReady
            select  localDrive.RootDirectory.FullName;

string path = String.Empty;

if (bestAPIDataDrives.Count() == 0)
{
    // there is only a system drive available, use it anyway.
    path = systemDrive;
}
else
{
    path = bestAPIDataDrives.ElementAt(0).ToString();
}

When I get to the bestAPIDataDrives.Count() reference, I get an IOException with Message property set to "The device is not ready".

I don't see why this is happening because I have the localDrive.IsReady test in the LINQ query, or at least I think I do.

joebalt
  • 969
  • 1
  • 12
  • 24
  • 2
    Marc has answered the real question, so two code review comments: This code actually executes the query twice, once to count and again to find the first element. You also do not have to ToString a string. Everything after building the query can be replaced with `string path = bestAPIDataDrives.FirstOrDefault() ?? systemDrive`. You also do not need to create a List from the results of `DriveInfo.GetDrives()`, and can say `IEnumerable localDrives = DriveInfo.GetDrives()`. – Chris Pitman May 18 '11 at 12:26
  • @Chris - good spot; I definitely agree - should use `FirstOrDefault()` here (and nothing else; that covers both `Count()` and `ElementAt()`, much more efficiently) – Marc Gravell May 18 '11 at 12:34
  • Good advice again, all. Many thanks. Will update the solution with these code review items. – joebalt May 18 '11 at 12:51

1 Answers1

4

that is LINQ-to-Objects, so it will execute the operations in the order presented; in particular, it will try to sort before it has filtered, and almost all of the filters execute before the key IsReady test. I would simply re-order them:

var bestAPIDataDrives = from localDrive in localDrives
            where   localDrive.IsReady &&
                    localDrive.DriveType == DriveType.Fixed &&
                    localDrive.DriveFormat == "NTFS" &&
                    localDrive.RootDirectory.FullName != systemDrive     
            orderby localDrive.AvailableFreeSpace descending                   
            select  localDrive.RootDirectory.FullName;

Running sorts after filters is a good idea anyway, as you reduce the cost of sorting (which is always super-linear)

As Chris notes, you then just want to finish up with:

string path = bestAPIDataDrives.FirstOrDefault() ?? systemDrive;
Marc Gravell
  • 1,026,079
  • 266
  • 2,566
  • 2,900