2

I have a method which reads all the files as a string from a directory and then iterate over those files to get file content along with other details and return a IEnumerable<DataFiles> object from it as shown below:

public IEnumerable<DataFiles> GetFiles(string path)
{
    var listOfFiles = new List<string>();
    try
    {
        var jsonDataFiles = Directory.GetFiles(path, "*.json", SearchOption.AllDirectories);
        var textDataFiles = Directory.GetFiles(path, "*.txt", SearchOption.AllDirectories);
        listOfFiles.AddRange(jsonDataFiles);
        listOfFiles.AddRange(textDataFiles);
        for (int i = 0; i < listOfFiles.Count; i++)
        {
            var cfgPath = listOfFiles[i];
            if (!File.Exists(cfgPath)) { continue; }
            var fileContent = File.ReadAllText(cfgPath);
            var pieces = cfgPath.Split(System.IO.Path.DirectorySeparatorChar, StringSplitOptions.RemoveEmptyEntries);
            var fileName = pieces[pieces.Length - 1];
            var md5HashValue = GetMD5Hash(cfgPath);
            // error on below line
            yield return new DataFiles
            {
                FileName = fileName,
                FileContent = fileContent,
                MD5Hash = md5HashValue
            };
        }
    }
    catch (UnauthorizedAccessException ex)
    {
        // log error here
    }
    // error on below line
    return null;
}

But somehow I get a compilation error at yield return new line and also return null line in my above method. Below are the errors I am seeing:

Cannot yield a value in the body of a try block with a catch clause
Cannot return a value from an iterator. Use the yield return statement to return a value, or yield break to end the iteration.

If above code throws exception then I want to return empty IEnumerable<DataFiles> object otherwise it should return proper IEnumerable<DataFiles> object` with data in it.

What is wrong I am doing here? Is there any better and clean way to write above method so that I don't have to return null at the end of the method?

cs98
  • 129
  • 2
  • 10
  • You should be able to just omit the `return null;` line if you're using `yield`. Otherwise have you tried `yield break` as per the error message? – Jamiec Aug 07 '20 at 15:34
  • But what happens if exception is thrown? – cs98 Aug 07 '20 at 15:35
  • Your iteration will stop... is that what you want? – Jamiec Aug 07 '20 at 15:35
  • I updated my question. I forgot to add that detail. I want to return empty `IEnumerable` object if exception is thrown basically otherwise return proper `IEnumerable` object with full content. – cs98 Aug 07 '20 at 15:37
  • Then dont use yield, because at the moment no error will be throw until the enumerator is evaluated. I'd suggest building up a `List` and return that or null. Its the use of `yield` which is confusing you here. – Jamiec Aug 07 '20 at 15:38
  • This is becuase `return` statement used inside the code block of for. – Abdul Haseeb Aug 07 '20 at 15:38
  • yeah but I want to return `IEnumerable` basically since it is explicitly telling caller that this value is readonly and not gonna add insert/delete anything here. – cs98 Aug 07 '20 at 15:39
  • Actually, cannot you just put everything outside the try-catch block except the calls to `Directory.GetFiles`? – Camilo Terevinto Aug 07 '20 at 15:39
  • `List` _is_ `IEnumerable` - or use `AsReadOnly()` – Jamiec Aug 07 '20 at 15:39
  • @CamiloTerevinto yeah I think I can but will it be the right way to do it? What happens if I see error while reading file from disk then I need to handle that also properly so thats why I put it in initial try catch block. In general how should I write this method in a proper way where I can return `IEnumerable` object back. – cs98 Aug 07 '20 at 15:41
  • Does this answer your question? [Why can't yield return appear inside a try block with a catch?](https://stackoverflow.com/questions/346365/why-cant-yield-return-appear-inside-a-try-block-with-a-catch) – Liam Aug 07 '20 at 15:41

2 Answers2

1

As Jamiec mentioned, you should be able to omit the 'return null' Unauthorized exception is usually a filesystem error, so grab all your files before hand. Also the second try catch block in case your processing goes wrong

public IEnumerable<DataFiles> GetFiles(string path)
    {
        var listOfFiles = new List<string>();
        try
        {
            listOfFiles.AddRange(System.IO.Directory.GetFiles(path, "*.json", System.IO.SearchOption.AllDirectories));
            listOfFiles.AddRange(System.IO.Directory.GetFiles(path, "*.txt", System.IO.SearchOption.AllDirectories));
        }
        catch (UnauthorizedAccessException ex)
        {
            // log error here
        }
        string fileName, fileContent;
        int md5HashValue; // im assuming this is an it, not sure
        //byte[] md5HashValue; // not sure
        for (int i = 0; i < listOfFiles.Count; i++)
        {
            try
            {
                var cfgPath = listOfFiles[i];
                if (!System.IO.File.Exists(cfgPath)) { continue; }
                fileContent = System.IO.File.ReadAllText(cfgPath);
                var pieces = cfgPath.Split(System.IO.Path.DirectorySeparatorChar, StringSplitOptions.RemoveEmptyEntries);
                fileName = pieces[pieces.Length - 1];
                md5HashValue = GetMD5Hash(cfgPath);
            }
            catch (Exception ex)
            {
                // log other error here
                continue;
            }
            // error on below line
            yield return new DataFiles
            {
                FileName = fileName,
                FileContent = fileContent,
                MD5Hash = md5HashValue
            };
        }
    
    }
Gauthier
  • 1,251
  • 10
  • 25
  • thanks for your suggestion. Is this the clean way to write above method? just making sure – cs98 Aug 07 '20 at 15:47
  • as to clean i cant say, just my prefered style. Also i don't normally write yield statements so, its possible that doesn't work out of the box. – Gauthier Aug 07 '20 at 15:48
  • as to all the extra System.IO. and namespaces. i hijacked one of the files im working on and didnt want to change that much. you can remove them as im guessing you already have those included. – Gauthier Aug 07 '20 at 15:50
  • got it now. I updated my question with my new code where I use List only but I marked it as `ReadOnly` at the end when I return it. Do you think that will also work without any issues? And my list that is returned at the end is `IEnumerable` but read only right? – cs98 Aug 07 '20 at 16:05
  • I would think that would work nicely (however usually about 30% of my assumptions bite me years later) I've never used the readonly, i would assume that is what will happen. I would write a test afterwards to double check, or find some sweet m-soft documentation. – Gauthier Aug 07 '20 at 16:09
  • sure also I wanted to make sure if somehow exception is throw then my method will return empty IEnumerable object right? I was just worried that on exception this line `dataFiles.AsReadOnly();` might throw exception so wanted to make sure. – cs98 Aug 07 '20 at 16:11
  • if an exception is thrown, you will have an empty read-only array, so when you call AsReadOnly it will return that empty array – Gauthier Aug 07 '20 at 16:14
  • Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/219421/discussion-between-cs98-and-gauthier). – cs98 Aug 07 '20 at 16:14
0

Do following steps:

  • declare typed variable for DataFiles outside try block
  • Assign data to that variable as:
yourDataFilesObj = new DataFiles
                {
                   FileName = fileName,
                   FileContent = fileContent,
                   MD5Hash = md5HashValue
                };

  • Outside Catch block write as
yield return yourDataFilesObj;
Sh.Imran
  • 1,035
  • 7
  • 13