Sign Up ›

Browse by Topic
ShowAll Questionssorted byDate Posted
+ Start a Discussion
sflearnsflearn 

more efficient for loop?

map should save us script limits

   try{
        for(Account a:aList){
               for(Contact c: cList){
                   if( c.custom_field__c == null 
                    || c.custom_field__c==null {
                       a.addError('missing contact info!);
                   }
               }      
            }
        }

        catch(System.Exception e){
            ex.setMessage('Something really bad!');
        }

Best Answer chosen by Admin (Salesforce Developers) 
sfdcfoxsfdcfox

There's not enough detail in this thread at all. If I start from the top down, here's how I might optimize the code (but remember, we don't even know what the objective is or what object trigger runs on, etc).

 

Map<Id,Account> aMap = new Map<Id,Account>();
aMap.putAll(aList);
for(Contact c:cList)
  if(c.custom_field__c==null||c.custom_field__c==null)
    aMap.get(c.accountid).addError('missing contact info!');

/////////////////////////////////////////////////////////////
for(Contact c:[SELECT Id,Name,AccountId FROM Contact WHERE AccountId IN :Trigger.new])
  if(String.isBlank(c.custom_field__c))
    Trigger.newMap.get(c.accountid).addError('missing contact info!');

/////////////////////////////////////////////////////////////
Map<Id,Contact[]> cMap = new Map<Id,Contact[]>();
for(Contact c:cList)
  cMap.put(c.AccountId,new Contact[0]);

for(Contact c:cList)
  cMap.get(c.accountId).add(c);

for(Account a:aList)
  if(cMap.containsKey(a.Id))
    for(Contact c:cMap.get(a.Id))
      if(c.custom_field__c==null||c.custom_field__c==null)
        a.addError('missing account info');

//////////////////////////////////////////////////////////////
for(OpportunityLineItem item:oliList)
  if(item.custom_field__c==null||item.custom_field__c==null)
   Trigger.newMap.get(item.opportunityid).addError('missing product info!');

You can't just add an error anywhere and expect the system to get it right. You have to trace it back to its source. When an account trigger queries contacts, it has to use Trigger.new or Trigger.newMap (or the old/oldMap for delete) in order to raise an error that will be trapped by the system. Notice how all the later examples use Trigger.newMap to correctly identify the correct record. This is how it should be.

 

As a general rule, this question was simply too vague to begin with; your attempt to abstract the idea to a basic pattern left out critical information that would have led to a more precise answer. For example, it would be good to know the context the code was running in. aList and cList could be from a trigger, a visualforce page, loaded from some class, or anything else. We just have no way of knowing, and that makes a correct answer more difficult, like "pin the tail on the donkey" or "break the pinata"; we're given blindfolds and expected to produce an answer.

 

However, from a generalization, you can be certain that any successful code between children and parents will almost certainly look like this:

 

map<id,parent> parents = new map<id,parent>( <parents from some source> );
list<child> children = new list<child>([select id,parentid,other_values__c from child where parentid in :parents.keyset());
for(child record:children)
  if( (record meets some criteria) )
    parents.get(record.parentid).adderror( ... );

 

All Answers

souvik9086souvik9086

Hi,

 

Nested for loops are not a best practice. What you can do is that

 

try{

List<Contact> conList = [SELECT ID,NAME,Account.name AccountID FROM Contact WHERE AccountId in : Trigger.new.id];

for(Contact con : conList){

if( con.custom_field__c == null || con.custom_field__c == ''){

con.Account.name.addError('missing contact info!);

}

}

}

catch(System.Exception e){
        ex.setMessage('Something really bad!');
}

 

AddError is needed there.

 

If this post is helpful please throw Kudos.If this post solves your problem kindly mark it as solution.

Thanks

 

 
sflearnsflearn

Thank you. I started playing around and I think this should work too but need to write my test methods. If I use a map with a lookup, it seems to save several code statements. More debugging is necessary but do u think this would be mre efficient then the single loop u have with a list. i will mark as solution once the thread is complete. thank you!

 

        Map<id, Contact> cMap= new Map<id, Contact>();

        for (Contact c: cList){
            cMap.put(c.AccountId, c);
        }

        for (Account a: aList){
            if (cMap.containsKey(a.id)){
                 if( cMap.get(a.id).custom_field__c== null 
                    ||  cMap.get(a.id).custom_field__c==null ){
                    a.addError('missing contact info!);
                    }
            }
        }
souvik9086souvik9086

Yes, correct. It needs to have morre test method but performance wise doesn't differ much. But the one you pointed here with map also looks good. It also doesn't need redundant loops. You can go ahead with it.

 

Thanks
sflearnsflearn

i also did this for opportunity but doesn';t throw...ummmm - do u see anything here. Trigger.new is passed into oList. my old code still works with the inner loop

 

 
souvik9086souvik9086

It would be like this

 

for(OpportunityLineItem lineitem: lineitemList){

 

What it is showing?

 

If the post is helpful please throw kudos.

Thanks.
sflearnsflearn

i mistyped in the thread but I am declaring it there as opportunitylineitem lineitem but for some reason the adderror is not firing on the opportunity parent record
sflearnsflearn

Ok. it was because soql did not have the Opportunity.Name in the query. however I am getting this error now which is because the record is outside of the context.

 

Apex trigger myTrigger caused an unexpected exception, contact your administrator: myTrigger: execution of BeforeUpdate caused by: System.FinalException: SObject row does not allow errors: Class.oppClass.LineItemCheck: line 26, column 1

 

this is the line:

lineitem.Opportunity.name.addError('Missing lineitem info!');

 

I think the map pattern is the way to go

 
Avidev9Avidev9

You can only add errors to the records that are currently referred in the trigger. So if the trigger is on OppLineItem you can do adderror to OppLineItem Only

 

May be you can try something like this

 

lineitem.OpportunityId.addError('Missing lineitem info!');

 Or

 

lineitem.addError('Missing lineitem info!');
sfdcfoxsfdcfox

There's not enough detail in this thread at all. If I start from the top down, here's how I might optimize the code (but remember, we don't even know what the objective is or what object trigger runs on, etc).

 

Map<Id,Account> aMap = new Map<Id,Account>();
aMap.putAll(aList);
for(Contact c:cList)
  if(c.custom_field__c==null||c.custom_field__c==null)
    aMap.get(c.accountid).addError('missing contact info!');

/////////////////////////////////////////////////////////////
for(Contact c:[SELECT Id,Name,AccountId FROM Contact WHERE AccountId IN :Trigger.new])
  if(String.isBlank(c.custom_field__c))
    Trigger.newMap.get(c.accountid).addError('missing contact info!');

/////////////////////////////////////////////////////////////
Map<Id,Contact[]> cMap = new Map<Id,Contact[]>();
for(Contact c:cList)
  cMap.put(c.AccountId,new Contact[0]);

for(Contact c:cList)
  cMap.get(c.accountId).add(c);

for(Account a:aList)
  if(cMap.containsKey(a.Id))
    for(Contact c:cMap.get(a.Id))
      if(c.custom_field__c==null||c.custom_field__c==null)
        a.addError('missing account info');

//////////////////////////////////////////////////////////////
for(OpportunityLineItem item:oliList)
  if(item.custom_field__c==null||item.custom_field__c==null)
   Trigger.newMap.get(item.opportunityid).addError('missing product info!');

You can't just add an error anywhere and expect the system to get it right. You have to trace it back to its source. When an account trigger queries contacts, it has to use Trigger.new or Trigger.newMap (or the old/oldMap for delete) in order to raise an error that will be trapped by the system. Notice how all the later examples use Trigger.newMap to correctly identify the correct record. This is how it should be.

 

As a general rule, this question was simply too vague to begin with; your attempt to abstract the idea to a basic pattern left out critical information that would have led to a more precise answer. For example, it would be good to know the context the code was running in. aList and cList could be from a trigger, a visualforce page, loaded from some class, or anything else. We just have no way of knowing, and that makes a correct answer more difficult, like "pin the tail on the donkey" or "break the pinata"; we're given blindfolds and expected to produce an answer.

 

However, from a generalization, you can be certain that any successful code between children and parents will almost certainly look like this:

 

map<id,parent> parents = new map<id,parent>( <parents from some source> );
list<child> children = new list<child>([select id,parentid,other_values__c from child where parentid in :parents.keyset());
for(child record:children)
  if( (record meets some criteria) )
    parents.get(record.parentid).adderror( ... );

 

This was selected as the best answer