Validate if a CPF already exists in the List before inserting C#

Asked

Viewed 561 times

2

Good evening, I am creating a sales system (college work) that has a simple registration of Clients (name and Cpf), we are working with the Project in layers, I am having difficulty because I am trying to validate if the CPF already exists in the list using FOREACH, but it doesn’t seem to be working because it’s accepting all the CPF’s and at the time I ask for a re-turn of the list, it’s not returning me anything... what’s wrong with the validation of Foreach? Thank you in advance!.

Code in C#

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using VendasOsorioA.Model;

namespace VendasOsorioA.DAL
{
    class ClienteDAO
    {

        private static List<Cliente> clientes = new List<Cliente>(); 
        public static void CadastrarCliente(Cliente c) 
        {
            foreach (Cliente ClienteCadastrado in clientes)
            {
                if (c.Cpf.Equals(ClienteCadastrado.cpf))
                {
                    Console.WriteLine("CPF JÁ EXISTE NA BASE, CADASTRO RECUSADO.");
                }
                else
                {
                    clientes.Add(c);
                    Console.WriteLine("CLIENTE CADASTRADO COM SUCESSO.");
                }
            }    
        }
        public static List<Cliente> RetornarClientes()
        {
            return clientes;
        }
    }

    }

3 answers

1

Hello! The problem is the logic you used. Your code is going through the list of registered customers, which is initially empty. How about this version?

        public static void CadastrarCliente(Cliente c)
        {
            foreach (Cliente ClienteCadastrado in clientes)
            {
                if (c.Cpf.Equals(ClienteCadastrado.Cpf))
                {
                    Console.WriteLine("CPF JÁ EXISTE NA BASE, CADASTRO RECUSADO.");
                    return;
                }
            }
            clientes.Add(c);
            Console.WriteLine("CLIENTE CADASTRADO COM SUCESSO.");
        }

0

I would recommend changing your code by doing a LINQ instead of a foreach(). This allows you to solve your problem and also simplifies the analysis of your code.

Do it like this:

private static List<Cliente> clientes = new List<Cliente>(); 

if (clientes.Any(cliente => cliente.cpf == cpf))
{
    clientes.Add(c);
    Console.WriteLine("CLIENTE CADASTRADO COM SUCESSO.");
}
else
{
    Console.WriteLine("CPF JÁ EXISTE NA BASE, CADASTRO RECUSADO.");
}
  • A pity not to show time, minute and second of the answers so that ignores like these do not happen. But all right, I understand your point of view.

  • Just keep the mouse pointer/mouse over the time pointers, which has exact details of each occurrence: https://i.stack.Imgur.com/Izxh3.png

  • Honestly, it doesn’t matter to me, it just matters if you helped the kid out there or not and somebody learned something from it. -2 rep for helping someone is a cheap price to pay yourself :)

  • So you notice the exchange of the word Singleordefault() for Any() and you don’t notice the difference between the comparison within the LINQ? No more...

  • I withdraw the accusation of copying with apology. I think it would be more constructive for Marconcilio to have given a suggestion of improvement with comment. However your answer still has information that is not correct. porque imagina quando sua base possuir milhares de dados, qual seria a performance do foreach() The Any has the same performance as foreach with data in memory

  • Apology accepted, it’s exactly what I thought, why he didn’t suggest an improvement. However Any() has a particularity that when it finds the first record specified by the clause, it stops its execution and already returns the result, not going through all the data.

  • Victor, this is exactly the algorithm with the foreach if implemented correctly.

  • In the end it will really give everything in the same, just take care with the mode that is implemented. But an Any() is not much more beautiful to see than a foreach()? rsrs

Show 3 more comments

0

One of the most efficient ways to do this is by using a HashSet and not a Lista. The HashSetis an implementation of a data structure that ensures no duplicated data in time O(1), unlike the Time List O(n) since in the worst case it is necessary to go through the list until the end to know whether or not the element is in it.

However you need to implement the method Equals and GetHashCode in the kind you keep in your HashSet, in this case would be in the type Cliente.

public class Cliente{
    //... Métodos, campos, propriedades, construtores etc

    public override bool Equals(object obj)
    {
        //sua implementação
    }

    public override int GetHashCode()
    {
        //sua implementação
    }
}

private static HashSet<Cliente> clientes = new HashSet<Cliente>(); 
public static void CadastrarCliente(Cliente c) 
{
    if(clientes.Add(c)){
        Console.WriteLine("CLIENTE CADASTRADO COM SUCESSO.");
    }else{
        Console.WriteLine("CPF JÁ EXISTE NA BASE, CADASTRO RECUSADO.");
    } 
}

Another thing that is wrong in your code is that you are changing the list within the foreach, normally doing so results in an exception.

  • 2

    I agree that his form is very effective using Hashset, but he is apparently starting in development, I think it will mess up his head even more. I think the best option right now is to let him understand the minimum first.

Browser other questions tagged

You are not signed in. Login or sign up in order to post.