+ Start a Discussion
Nagarjuna Reddy.PNagarjuna Reddy.P 

trigger to update ownership

Hi Everyone,

I have a requirement as follows
When Case ownership is updated
1.update case related Account and account related contact ownership with new owner of Case.
2. update case related contact ownership with new owner of case

Here is my code..
public class caseTriggerExamples {

  public static void caseOwner(Map<id,Case> oldMap, Map<id,Case> newMap){

      List<id> accids=new List<id>();
      List<id> conids= new List<id>();
      List<Case> cases = new List<Case>();
       for(id key:oldMap.keyset()){
            Case oldCase = oldMap.get(key);
            Case newCase = newMap.get(key);
            if(oldCase.ownerid!=newCase.Ownerid){
            accids.add(newCase.AccountId);
                conids.add(newCase.ContactId);
            cases.add(newCase);
         }
      }
  List<Account> accounts = [select id,name,ownerid,(select id, Ownerid from contacts) from Account where id in:accids];
  List<Contact> contacts= [select id,ownerid from contact where id in:conids];
       List<Contact> cons= new List<Contact>();
       for(Account a:accounts){
           for(contact con:a.contacts){
               for(Case c: cases){
                 a.ownerid=c.ownerid;
                   con.OwnerId = c.OwnerId;
                   cons.add(con);
               }                   
        }

     }
      for(Contact con:contacts){
          for(Case ca:cases){
              con.OwnerId=ca.OwnerId;
          }
      }
         update accounts;
         update contacts;
         update cons;
       }
   }

trigger caseTriggerExamples on Case (after update) {
   if(trigger.isUpdate && trigger.isAfter){
      caseTriggerExamples.caseOwner(trigger.oldMap,trigger.newMap);
   }
}

My code is working fine, but my question here is, Is there any possibility of reducing using multiple for loops and achieve my requirement. This is the question asked in a recent interview and he asked me not to include multiple for loops.

Any help is appreciated. Thank you!!
 
Best Answer chosen by Nagarjuna Reddy.P
RituSharmaRituSharma
There are lot of things that you may optimize/fix in this code. However, you may avoid the trigger and achieve the requirement using flow/process builder easily. 

1. Below line of code will fail if any of the queried account is having more than 200 contacts. It's a limitation of inner queries. I don't think that you really need to query accounts.
List<Account> accounts = [select id,name,ownerid,(select id, Ownerid from contacts) from Account where id in:accids];

2. In below line of code, you are querying contacts again. You queried contacts in account query too. I don't think that you really need to query contacts.
List<Contact> contacts= [select id,ownerid from contact where id in:conids];

3. In below logic, you have 3 loops(one inside another) but you are trying to just update cases. You can directly loop on cases and create an instance of the account and contact and then put them in a list.

Existing Code
for(Account a:accounts){
           for(contact con:a.contacts){
               for(Case c: cases){
                 a.ownerid=c.ownerid;
                   con.OwnerId = c.OwnerId;
                   cons.add(con);
               }                   
        }

Suggested Code - Will fit for entire logic
List<Account> accList = new List<Account>();
List<Contact> conList = new List<Contact>();
for(Cases cs: Cases) {
    Account acc = new Account(Id=cs.AccountId,OwnerId=cs.OnwerId);
    accList.add(acc);
    Contact con = new Contact(Id=cs.ContactId,OwnerId=cs.OnwerId);
    conList.add(con);
}
update conList;
update accList;

All Answers

RituSharmaRituSharma
There are lot of things that you may optimize/fix in this code. However, you may avoid the trigger and achieve the requirement using flow/process builder easily. 

1. Below line of code will fail if any of the queried account is having more than 200 contacts. It's a limitation of inner queries. I don't think that you really need to query accounts.
List<Account> accounts = [select id,name,ownerid,(select id, Ownerid from contacts) from Account where id in:accids];

2. In below line of code, you are querying contacts again. You queried contacts in account query too. I don't think that you really need to query contacts.
List<Contact> contacts= [select id,ownerid from contact where id in:conids];

3. In below logic, you have 3 loops(one inside another) but you are trying to just update cases. You can directly loop on cases and create an instance of the account and contact and then put them in a list.

Existing Code
for(Account a:accounts){
           for(contact con:a.contacts){
               for(Case c: cases){
                 a.ownerid=c.ownerid;
                   con.OwnerId = c.OwnerId;
                   cons.add(con);
               }                   
        }

Suggested Code - Will fit for entire logic
List<Account> accList = new List<Account>();
List<Contact> conList = new List<Contact>();
for(Cases cs: Cases) {
    Account acc = new Account(Id=cs.AccountId,OwnerId=cs.OnwerId);
    accList.add(acc);
    Contact con = new Contact(Id=cs.ContactId,OwnerId=cs.OnwerId);
    conList.add(con);
}
update conList;
update accList;
This was selected as the best answer
Nagarjuna Reddy.PNagarjuna Reddy.P
Hi Ritu,

Very big thanks for your reply with code optimization, it really works well. your suggested code updates case relaated accounts and contacts. 
Here I have one more update to perform i.e account related contacts, So  if i query account and contacts like
List<Account> accounts = [select id,name,ownerid,(select id, Ownerid from contacts) from Account where id in:accids];
we may get internal query limitation error , what is the best way to do then? 
RituSharmaRituSharma
Directly query on contacts using accids. Loop on queried contacts to prepare a map where key is account id and value is list of contacts. You may then use this map inside case loop to get the contacts(based on account key). You may then add these contacts to conList.