select random file from directory

Question

Any suggestions on how to improve this method? I am currently using it to select a single wallpaper from a directory of wallpapers

I know your not supposed to use arraylist anymore but i couldnt think of a altrnative also im not sure how to filter for more than just one type of file (ie jpg gif png) in the directory info.

any suggestions or tweaks would be fantastic

private string getrandomfile(string path)
        {
            ArrayList al = new ArrayList();
            DirectoryInfo di = new DirectoryInfo(path);
            FileInfo[] rgFiles = di.GetFiles("*.*");
            foreach (FileInfo fi in rgFiles)
            {
                al.Add(fi.FullName);
            }

            Random r = new Random();
            int x = r.Next(0,al.Count);

            return al[x].ToString();

        }

Thanks

Crash

Answer 1

Do you really need the ArrayList at all, you should be able to eliminate it and just use the array directly once you've generated a random number.

Also, you should check that the path is valid... if specified by a user...

Answer 2

Changed to use a single instance of the pseudo-random number generator.

// Use a class variable so that the RNG is only created once.
private Random generator;
private Random Generator
{
    get
    {
        if (this.generator == null)
        {
           this.generator = new Random();
        }
        return this.generator;
    }
}
private string getrandomfile(string path)
{
    string file = null;
    if (!string.IsNullOrEmpty(path))
    {
        var extensions = new string[] { ".png", ".jpg", ".gif" };
        try
        {
            var di = new DirectoryInfo(path);
            var rgFiles = di.GetFiles("*.*")
                            .Where( f => extensions.Contains( f.Extension
                                                               .ToLower() );
            int fileCount = rgFiles.Count();
            if (fileCount > 0)
            {
                int x = this.Generator.Next( 0, fileCount );
                file = rgFiles.ElementAt(x).FullName;
            }
        }
        // probably should only catch specific exceptions
        // throwable by the above methods.
        catch {}
    }
    return file;
}

Answer 3

Why not use LINQ:

var files = Directory.GetFiles(path, "*.*").Where(s => Regex.Match(s, @"\.(jpg|gif|png)$").Success);
string randFile = path + files.ToList()[r.Next(0, files.Count())];

Answer 4

As always - there is more than one way to skin a cat. I've built off of tvanfosson (correct) answer, not because this is 'more' correct; but because I think it's a useful approach.

private static string getRandomFile(string path)
{
    try
    {
        var extensions = new string[] { ".png", ".jpg", ".gif" };

        var di = new DirectoryInfo(path);
        return (di.GetFiles("*.*")
                            .Where(f => extensions.Contains(f.Extension
                                                               .ToLower()))
                            .OrderBy(f => Guid.NewGuid())
                            .First()).FullName ;              
    }
    catch { return ""; }
}

Answer 5

I made a few changes

here is the code i ended up using, I cut out some of the conditonals becuase they dont really matter ( if there are no files it will return null anyway no need to test twice). I also corrected for a few minor syntax errors and one user pointed out the return should be moved down.

also in regards to the random class, Im not sure why it was bad to keep calling it but i dont see that its necessary since this will only run once every 10 to 15 min. and even then it would only create the class if files were found.

Thanks for everyone's help ( tvanfosson)

private string getrandomfile2(string path)
    {
        string file = null;
        if (!string.IsNullOrEmpty(path))
        {
            var extensions = new string[] { ".png", ".jpg", ".gif" };
            try
            {
                var di = new DirectoryInfo(path);
                var rgFiles = di.GetFiles("*.*").Where( f => extensions.Contains( f.Extension.ToLower()));
                Random R = new Random();
                file = rgFiles.ElementAt(R.Next(0,rgFiles.Count())).FullName;
            }
            // probably should only catch specific exceptions
            // throwable by the above methods.
            catch {}
        }
        return file;
    }

Answer 6

please delete this post