CODEWITHSUNDEEP
c#
WpfBee
WpfBee

Reputation: 2927

How to apply if condition in multiple statements, as a best practice?

I need to apply If condition in multiple statements like below.

Two questions:

  1. Any best practice to get it done?

  2. Returning control from foreach loop like below: is it safe doing so?

    if (assettFileParser.AddFile(_cfgFile, MyFileType.CFG) == ParserReturnCode.FileNotFound) return false;
    if (_psFiles != null)
         foreach (var file in _psFiles)
                  if(assettFileParser.AddFile(file, MyFileType.PS)== ParserReturnCode.FileNotFound) return false;
    if (_picFiles != null)
         foreach (var file in _picFiles)
                  if (assettFileParser.AddFile(file, MyFileType.ParaIC) == ParserReturnCode.FileNotFound) return false;
    if (_icFiles != null)
        foreach (var file in _icFiles)
                 if (assettFileParser.AddFile(file, MyFileType.IC) == ParserReturnCode.FileNotFound) return false;
    if (_xMasterFiles != null)
        foreach (var file in _xMasterFiles)
                 if (assettFileParser.AddFile(file, MyFileType.XMaster) == ParserReturnCode.FileNotFound) return false;

Upvotes: 0

Views: 96

Answers (1)

Georg Patscheider
Georg Patscheider

Reputation: 9463

Here I combine the suggestions of Dylan Nicholson and Poylfun: use a helper method that throws an exception (you can introduce your own exception type by inheriting from System.Exception). I also moved the foreach into the helper method, it accepts a collection of files. The helper also makes sure that the collection of files is not null, so the caller does not have to check this.

public void AddAssetFiles(AssettFileParser assettFileParser,
                          IEnumerable<string> files, MyFileType fileType) {
   if (files == null) { 
       return; // do nothing
   }
   // note that looping over an empty collection will do nothing
   foreach (var file in files) {
        var returnCode = assettFileParser.AddFile(file, fileType); 
        if (returnCode == ParserReturnCode.FileNotFound) {
            throw new AssetFileNotFoundException($"File '{file}' was not found.");
        }
   }
}

Usage example. First file not found will result in termination of the whole try block. No further files are processed afterwards. 

try {
    // put _cfgFile into a string array so it can be passed as IEnumerable<string>
    AddAssetFiles(assettFileParser, new string[] { _cfgFile }, MyFileType.CFG);
    AddAssetFiles(assettFileParser, _psFiles, MyFileType.PS);
    AddAssetFiles(assettFileParser, _picFiles, MyFileType.ParaIC);
    // ...
    return true;
}
catch (AssetFileNotFoundException ex) {
    Logger.Error(ex);
    return false;
}

Side note on code style: you should always put if and foreach blocks into brackets { }, even if the block contains only a single line. This can forestall problems with dangling else etc.

Upvotes: 1

Related Questions