Organization for future code maintenance when I have two similar functions

Question

I have two functions, one returns only the first result and the other also returns the first result or returns more than one result.

I created the "string all" parameter to define whether to return one or more results in the second function. But the second function became more complex and I could have made it simpler to work just to return several results

I must continue programming complex equal to the second function or make it simpler and with only one goal?

Test call:

    Pastas pastas1 = new Pastas(@"C:\New folder\Nova pasta\");
    Pastas.Teste1(pastas1, "3.txt");

Code:

// Mapeia todo as pastas e subpastas de um diretório
class Pastas
{ 
    public DirectoryInfo Diretorio;
    public List<Pastas> SubDiretorios = new List<Pastas>();
    public FileInfo[] Arquivos;

    public Pastas(string pasta)
    {
        Diretorio = new DirectoryInfo(pasta);

        foreach (DirectoryInfo di in Diretorio.GetDirectories())
        {
            SubDiretorios.Add(new Pastas(di.FullName));
        }

        Arquivos = Diretorio.GetFiles();
    }
// Primeira Função
// Percorre as pastas e retorna o primeiro arquivo pelo nome
    public FileInfo GetArquivo(string nome)
    {
        FileInfo arquivo = Arquivos.FirstOrDefault(x => x.Name == nome);
        if (arquivo is null)
        {
            foreach (Pastas item in SubDiretorios)
            {
                arquivo = item.GetArquivo(nome);
                if (arquivo != null)
                    break;
            }
        }

        return arquivo;
    }
// Segunda função
    public List<FileInfo> GetArquivo(string nome, bool todos)
    {
        List<FileInfo> arquivos = new List<FileInfo>();

        if (todos)
            arquivos = Arquivos.Where(x => x.Name == nome).ToList();
        else
        {
            FileInfo arquivo = Arquivos.FirstOrDefault(x => x.Name == nome);
            if (arquivo != null)
              arquivos.Add(arquivo);
        }

        if (todos || arquivos.Count == 0)
        {
            foreach (Pastas item in SubDiretorios)
            {
                arquivos.AddRange(item.GetArquivo(nome, todos));

                bool apenasOPrimeiro = !todos;
                if (apenasOPrimeiro && arquivos.Count == 1)
                    break;
            }
        }

        return arquivos;
    }

    public static void Teste1(Pastas pastas, string nomearquivo)
    {
        FileInfo arquivo = pastas.GetArquivo(nomearquivo);
        if (arquivo != null)
            Debug.Print(arquivo.FullName);
        else
            Debug.Print("arquivo nulo");
    }

    public static void Teste2(Pastas pastas, string nomearquivo)
    {
        List<FileInfo> arquivos = pastas.GetArquivo(nomearquivo, true);
        if (arquivos.Count > 0)
            arquivos.ForEach(x => Debug.Print(x.FullName));
        else
            Debug.Print("arquivo nulo");
    }

    // GetArquivo(..., false)
    public static void Teste3(Pastas pastas, string nomearquivo)
    {
        List<FileInfo> arquivos = pastas.GetArquivo(nomearquivo, false);
        if (arquivos.Count > 0)
            arquivos.ForEach(x => Debug.Print(x.FullName));
        else
            Debug.Print("arquivo nulo");
    }
}

Answer 1

What you must do only you can decide.

I always prefer the simple, without being simple. It is not always easy to say what is simple. And you have to look at other things, in some cases it’s best to make it a little more complicated if you have some clear advantage in doing this.

I I hate this idea of passing a boolean to tell you what to do (although there are cases for use). I even asked a question about this: Why not use a boolean parameter?.

In this case it’s not just the boolean, even use a enum more semantic I don’t like. NET does it and I don’t like it. But it’s not the end of the world to use. I prefer this to violate the DRY, for example, but there are ways to have more than one method, one for single result and one for multiple result without violating DRY, although it may get a little more complex.

This case of yours seems to me that having both methods is ideal, and the parameter todos nor should there be.

Note that LINQ does just this, has a method to pick up everything or to pick up only the first and the other to pick up the default if I can’t even get that.

Another not too bad possibility is to make a method that follows the same LINQ systematic and return an enumerator and then if you want only one or several can compose one query to do it efficiently. To tell the truth in the form created would be the best to do. But I know you probably won’t be able to do this, I’m putting it because maybe it’s the way I would.

It has many solutions, but they are not perfect.

The code has several other problems. The use of .ToList() then it seems wrong. Are you sure you need to do this? If you don’t know how to use LINQ correctly do it the normal way without it. Just call the ToList() If you want to put the list into practice, that does not seem to be necessary. The solution is also not quite what was put in the other answer, but anyway, I will not be able to solve everything here.

The second if seems not to be necessary, it should be within the first (if the todos). That one arquivos.Count == 0 it seems strange there, mainly because then there is a arquivos.Count == 1, but I can’t even say without understanding what you need. This flag apenasOPrimeiro seems unnecessary and only exists because the code is confusing. The point is that this code can certainly be written in a simpler way.

I’m afraid of these guys Pastas and List, This feels like something done wrong.

Answer 2

I decided to use the hint of this link and the second function: Why not use a boolean parameter?

That would continue to use the "bool todos" parameter, but create functions that call this main function.

Here is the simplest and improved code.

    private List<FileInfo> GetArquivos(string nome, bool todos)
    {
        List<FileInfo> arquivos = new List<FileInfo>();

        // Forma antiga com o ToList()
        // Conceito do ToList(): é utiliado quando consolidar dados
        //arquivos = Arquivos.Where(x => x.Name == nome).ToList();

        foreach (FileInfo item in Arquivos)
        {
            if (item.Name == nome)
                arquivos.Add(item);
        }

        if (!todos && arquivos.Count > 0)
            return arquivos;

        foreach (Pastas item in SubDiretorios)
        { 
            arquivos.AddRange(item.GetArquivos(nome, todos));
        }

        return arquivos;
    }

    public List<FileInfo> GetArquivosPeloNome(string nome)
    {
        return GetArquivos(nome, true);
    }

    public FileInfo GetPrimeiroArquivoPeloNome(string nome)
    {
        return GetArquivos(nome, false).FirstOrDefault();
    }