+ Start a Discussion
anup-prakashanup-prakash 

Is this Trigger okay to check for duplicate name

Hi.

 

   I have written just a few triggers and was wondering if this trigger is absolutely fine or if it lacks something.. although the trigger is working when i am inserting single entry through the window.. was wondering if it would also work when using data Loader.

Best Answer chosen by Admin (Salesforce Developers) 
JayNicJayNic

This should work.

 

One thing you needed to do was check in the list of inserts/updates that are being submitted if there are dupes in there as well. Otherwise I could batch insert two products with the same name and your trigger wouldn't catch me

 

trigger CheckDuplicateProduct on Prod__c (before insert, before update) {
	/*Processes:
		Check for duplicate names
	*/
	
	//We ahve two things to do that must be done in this order:
	//	ensure there are no duplicates in the list of new updates, or inserts
	//	then ensure there are no duplicates in the database
	
	map<string,Prod__c> names = new map<string,Prod__c>();
	for(Prod__c p : trigger.new) {
		string name = p.name.toLowerCase();
		if(names.get(name) == null) {
			//This is not a duplicate
			names.put(name, p);
		} else {
			//Wups! we are submitting two of the same name!
			p.addError('Duplicate Product names detected');
		}
	}
	
	//Now let's check the database for duplicates as well
	list<Prod__c> products = [SELECT name FROM Product WHERE name IN :names.keyset()];
	
	for(prod__c p : prods) {
		string name = p.name.toLowerCase();
		if(names.get(name) != null) {
			names.get(name).addError('A product with that name already exists in the database');
		}
	}
	
}

 

 

All Answers

JayNicJayNic
You forgot to paste the code
anup-prakashanup-prakash
trigger CheckDuplicateProduct on Prod__c (before insert, before update) {
set<String> prodNewName;
map<ID,String> prodName;

for(Prod__c p: Trigger.new){
prodNewName.add(p.Name);
}

System.debug(prodNewName);

for(Prod__c p: [select Name from Prod__c where Name IN :prodNewName]){
prodName.put(p.ID, p.Name);
}

if(prodName.size() == 0 ){
for(Prod__c p: Trigger.new){
p.addError('Product By This Name already exisits');
}
}
}



I am so sorry...
anup-prakashanup-prakash
Its not letting me insert through browser as well.. indivisually
Sean TanSean Tan

I'm pretty sure your trigger will force records that aren't dupes to fail (during bulk operations) if there is a record that actually has a dupe. Since you're not  remapping it.

 

Try something like this:

 

trigger CheckDuplicateProduct on Prod__c (before insert, before update) {    
    Map<String, Prod__c> prodMap = new Map<String, Prod__c>{};    
    
    for(Prod__c p: Trigger.new)
    {
        //If inserting or the name is changing
        if (Trigger.isInsert || Trigger.oldMap.get(p.Id).Name != p.Name)
        {
            prodMap.put(p.Name, p);
        }
    }
            
            
    if (!prodMap.isEmpty())
    {
        for(Prod__c p: [select Name from Prod__c where Name IN :prodMap.keySet()]) {        
            Prod__c changedProd = prodMap.get(p.Name);
            
            if (changedProd != null)
            {
                changedProd.addError('Product By This Name already exisits');
            }        
        }
    }
}

 

anup-prakashanup-prakash
trigger CheckDuplicateProduct on Prod__c (before insert, before update) {
	set<String> prodNewName = new set<String>();
	map<ID,String> prodName = new map<ID,String>();
	
	for(Prod__c p: Trigger.new){
		if (Trigger.isInsert || Trigger.oldMap.get(p.Id).Name != p.Name){
			prodNewName.add(p.Name);
		}
	}
	
	System.debug(prodNewName);
	
	for(Prod__c p: [select Name from Prod__c where Name IN :prodNewName]){
		prodName.put(p.ID, p.Name);
	}
	
	if(!(prodName.isEmpty())){
		for(Prod__c p: Trigger.new){
			p.addError('Product By This Name already exisits');
		}
	}
}

 is this piece of code fine now? or should I stick to the one you provided? and whats the difference?

Sean TanSean Tan

I would try to use what I provided ... (It also reduces script statements).

 

To give you an explanation as to what the difference is, the revised code you posted doesn't handle the scenario of multiple records.

 

Let's say you have the following being inserted:

 

Record 1

Record 2

 

You end up doing the following:

 

  • You query the database to see if there are any prods with the name Record 1 or Record 2 ([select Name from Prod__c where Name IN :prodNewName])
  • From there you check did anything match. (if(!(prodName.isEmpty())){)
  • If so add an error on every record being processed in the trigger in that context.

However, what happens if only ONE record in the above has a dupe, the query will still return one record... the isEmpty check will not handle your business case correctly since the way you're logic is being done is an all or nothing scenario.

 

JayNicJayNic

This should work.

 

One thing you needed to do was check in the list of inserts/updates that are being submitted if there are dupes in there as well. Otherwise I could batch insert two products with the same name and your trigger wouldn't catch me

 

trigger CheckDuplicateProduct on Prod__c (before insert, before update) {
	/*Processes:
		Check for duplicate names
	*/
	
	//We ahve two things to do that must be done in this order:
	//	ensure there are no duplicates in the list of new updates, or inserts
	//	then ensure there are no duplicates in the database
	
	map<string,Prod__c> names = new map<string,Prod__c>();
	for(Prod__c p : trigger.new) {
		string name = p.name.toLowerCase();
		if(names.get(name) == null) {
			//This is not a duplicate
			names.put(name, p);
		} else {
			//Wups! we are submitting two of the same name!
			p.addError('Duplicate Product names detected');
		}
	}
	
	//Now let's check the database for duplicates as well
	list<Prod__c> products = [SELECT name FROM Product WHERE name IN :names.keyset()];
	
	for(prod__c p : prods) {
		string name = p.name.toLowerCase();
		if(names.get(name) != null) {
			names.get(name).addError('A product with that name already exists in the database');
		}
	}
	
}

 

 

This was selected as the best answer
anup-prakashanup-prakash

thanks So muchh.. fo letting me know the difference. I'l do a deep dive into it and try to understand the functioning. But nevertheless thanks sooo Muchh..

JayNicJayNic
Both anups, and seans are good, but they fail to check against names being submitted in the current transaction.
You specifically asked if it could be used via the data loader, with mine you can because it checks all records being submitted.

With the other solutions, you could load an entire spreadsheet of the same name, and the system wouldn't catch it.
anup-prakashanup-prakash
The trigger that you had given doesn't allow me to enter records into the related List..
Sean TanSean Tan

Are you saying the trigger stops you from adding anymore Prod__c records?

 

Can you post the code that you used for the final trigger?

anup-prakashanup-prakash
Sean Your Trigger is working perfect..
But the other one of JayNic is not allowing me.. So your trigger is absolutely fine right?? Jaynic was saying that It'll face problem through dataLoader.. is it so? I am in dilemma..
Sean TanSean Tan

Well, here's a modified version of my trigger that will handle that one case that JayNic pointed out.

 

One thing to note about triggers is, Ideally you want your triggers to only ever run if you need to do something with it (in the case of updates of records you don't need to check for dupes if the name isn't changing since you're before insert already validates dupes).

 

trigger CheckDuplicateProduct on Prod__c (before insert, before update) {    
    Map<String, Prod__c> prodMap = new Map<String, Prod__c>{};    
    
    for(Prod__c p: Trigger.new)
    {
        //If inserting or the name is changing
        if (Trigger.isInsert || Trigger.oldMap.get(p.Id).Name != p.Name)
        {
            if (prodMap.containsKey(p.Name))
            {
                p.addError('Duplicate product names detected. Dupes will be ignored');
            }
            else
            {
                prodMap.put(p.Name, p);
            }
        }
    }
            
            
    if (!prodMap.isEmpty())
    {
        for(Prod__c p: [select Name from Prod__c where Name IN :prodMap.keySet()]) {        
            Prod__c changedProd = prodMap.get(p.Name);
            
            if (changedProd != null)
            {
                changedProd.addError('Product By This Name already exisits');
            }        
        }
    }
}

 

anup-prakashanup-prakash
Thanks Sean..
anup-prakashanup-prakash
trigger CheckDuplicateProduct on Prod__c (before insert, before update) I {

	set<String> setProdName = new  set<String>();  // Set of new Names that are ready for Insert
	
		for(Prod__c prod: Trigger.new){  //trigger.new gives list of new records of prod__c
			if(trigger.isInsert){
				setProdName.add(prod.Name);
			}
			if(trigger.isUpdate){
				if(!(trigger.oldMap.get(prod.ID).Name.equalsIgnoreCase(prod.Name))){
					setProdName.add(prod.Name);
				}
			}
		}
		
		List<Prod__c> listProdInDatabase = [select Name from Prod__c where Name in :setProdName];
		
		for(Prod__c prod : Trigger.new){
			for(Prod__c p : listProdInDatabase){
				if(prod.Name.equalsIgnoreCase(p.Name)){
					prod.addError('Duplicate Entry By Product Name');
				}
			}
		}		
}

 I understood Your Logic.. and Could you check for the last time as to what I have Written If this is okay?

Sean TanSean Tan

Couple of things to compare...

 

Here's the latest revision of mine to ignore case sensitivity within the Map.

 

trigger CheckDuplicateProduct on Prod__c (before insert, before update) {    
    Map<String, Prod__c> prodMap = new Map<String, Prod__c>{};    
    
    for(Prod__c p: Trigger.new)
    {
        //If inserting or the name is changing
        if (Trigger.isInsert || Trigger.oldMap.get(p.Id).Name != p.Name)
        {
	    String prodName = p.Name.toLowerCase();
            if (prodMap.containsKey(prodName))
            {
                p.addError('Duplicate product names detected. Dupes will be ignored');
            }
            else
            {
                prodMap.put(prodName, p);
            }
        }
    }
            
            
    if (!prodMap.isEmpty())
    {
        for(Prod__c p: [select Name from Prod__c where Name IN :prodMap.keySet()]) {        
            Prod__c changedProd = prodMap.get(p.Name.toLowerCase());
            
            if (changedProd != null)
            {
                changedProd.addError('Product By This Name already exisits');
            }        
        }
    }
}

Now for you're trigger. A couple of things:

 

  1. The if (Trigger.isInsert) vs if (Trigger.isUpdate), if you want to write the code that way, use an else if statement as you can only do one or the other. I'm not sure why you want to split it into two conditions, vs one if with an OR though.
  2. The == clause when comparing strings is the same as equalsIgnoreCase (in Apex that is). So you can simply use == instead.
  3. Why are you not using a Map to maintain the prods to the Prod name. It will prevent excessive looping at the bottom.

Other then that, you're logic looks fine.

anup-prakashanup-prakash
Thanks Sean I got It.. Thanks So Much.. :)