Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have a piece of code that checks if 3 specifically named files are present or not.

string[] filePaths = FilterFiles(ProgramSettings.DFLogPath, "*.log", DateTime.Now, "yyyy_MM_dd");
        filePaths = filePaths.Where(file => new FileInfo(file).Name.IndexOf("validate_NewMasterFile", StringComparison.OrdinalIgnoreCase) > -1 || new FileInfo(file).Name.IndexOf("validate_AuthFile", StringComparison.OrdinalIgnoreCase) > -1 || new FileInfo(file).Name.IndexOf("TransactionDatafeedLogFile", StringComparison.OrdinalIgnoreCase) > -1).ToArray<string>();
        if (filePaths.Length != 3)
        {
            LogThis("No. of files found: " + filePaths.Length, "r");

            LogThis("Files found:", "r");
            foreach (string filePath in filePaths)
                LogThis(new FileInfo(filePath).Name, "r");

            return false;
        }
        else
        {
            string validate_NewMasterFile = "";
            string validate_AuthFile = "";
            string TransactionDatafeedLogFile = "";

            foreach (string filePath in filePaths)
            {
                if (filePath.IndexOf("validate_NewMaster", StringComparison.OrdinalIgnoreCase) > -1)
                    validate_NewMasterFile = new FileInfo(filePath).Name;
                else if (filePath.IndexOf("validate_Auth", StringComparison.OrdinalIgnoreCase) > -1)
                    validate_AuthFile = new FileInfo(filePath).Name;
                else if (filePath.IndexOf("TransactionDatafeedLog", StringComparison.OrdinalIgnoreCase) > -1)
                    TransactionDatafeedLogFile = new FileInfo(filePath).Name;
            }

            LogThis("All files found", "g");
            LogThis("Master DataFeed log: " + validate_NewMasterFile, "g");
            LogThis("Auth DataFeed log: " + validate_AuthFile, "g");
            LogThis("Transaction DataFeed log: " + TransactionDatafeedLogFile, "g");
        }

There should be only 3 files in the folder (one of each name). The second line of code can return 3 even if there are 3 files named "TransactionDatafeedLog". It does not make sure that for each name, there is only one file.

Please notice that in the else part, I had to iterrate through the files to make sure that the all 3 files are present. I can check the strings (ex TransactionDatafeedLog), if its empty than its not present. If there are 2 or more files named like TransactionDatafeedLog, then the loop will select the last file and will not notice it as an error.

In short what I need is:

  • For each name there should be one and only one file.

It will be an error if:

  • If there are multiple files for any name.
  • If each and every file is not present. If any file is missing, I want to Log that too.

The code is very long and verbose and does not do what is should. Need help.

share|improve this question

closed as off topic by Glenn Rogers, codesparkle, sepp2k Jan 2 '13 at 15:10

Questions on Code Review Stack Exchange are expected to relate to code review request within the scope defined by the community. Consider editing the question or leaving comments for improvement if you believe the question can be reworded to fit within the scope. Read more about reopening questions here.If this question can be reworded to fit the rules in the help center, please edit the question.

    
If the code does not work, then this is a wrong site according to Q & A. I did not understand what exactly this code should be doing. [Mod note: removed unproductive bits of the comment] –  Leonid Jan 2 '13 at 3:03
    
Either 1) Split the code apart so you check one file at a time, or 2) Sort filePaths so you know what ..[0] etc should be –  Glenn Rogers Jan 2 '13 at 8:05
    
I'm sorry, but this site is for getting reviews of working code only, so this is off topic. –  sepp2k Jan 2 '13 at 15:10

1 Answer 1

up vote 2 down vote accepted

Seems as though you are complicating things by trying to find all log files at once. I would recommend writing a generic routine that searches for these files one at a time and then call it for the three different patterns separately.

Something like this.. (disclaimer: written in NotePad)

NOTE: Not exactly sure what FilterFiles does but since you can put "*.log" as a parameter I am assuming this parameter is a search pattern"

string validate_NewMasterFile = GetLogFileName("validate_NewMaster");
if (string.IsNullOrWhitespace(validate_NewMasterFile)
    return false;

string validate_Auth= GetLogFileName("validate_Auth");
if (string.IsNullOrWhitespace(validate_Auth)
    return false;

string transactionDatafeedLog = GetLogFileName("TransactionDatafeedLog");
if (string.IsNullOrWhitespace(transactionDatafeedLog )
    return false;

private bool GetLogFileName(string pattern)
{
    string[] filePaths = FilterFiles(ProgramSettings.DFLogPath, pattern + "*.log", DateTime.Now, "yyyy_MM_dd");

    // Expect only one file matching search pattern
    if (filePaths.Length != 1)
    {
        LogThis("Too many files found. Number found is: " + filePaths.Length);
        return null;
    }

    string fileName = new FileInfo(filePath[0])).Name;
    LogThis("File found: " + fileName, "r");

    return fileName;
}
share|improve this answer

Not the answer you're looking for? Browse other questions tagged or ask your own question.