function readOnly(count){ }
Starting November 20, the site will be set to read-only. On December 4, 2023,
forum discussions will move to the Trailblazer Community.
+ Start a Discussion
RelaxItsJustCodeRelaxItsJustCode 

Need help with for loops, Please help I will give kudos.

I have 2 loops that are causing some small problems.

 

In the code below you will note that I have two loops that hit the PriceBookEntry table (pbe1).  The first loop searches through out main production pricebook.  The second loop, loops through the PriceBookEntry table (pbe2) searching to find the productid in the standard pricebook.

 

The  PROBLEM:

 

When the first loop finds the productId I am looking for and grabs the listprice out of the main pricebook, it sometimes continues to loop through the second loop through the standard and then the update has two duplicate id's and blows to high hell and back.

 

Anyone that contributes will get kudos.

 

The person that gets me closest to the solution will get solution credit as well.

 

Here's the code:

 

public with sharing class MaintContractLineItemCreation 
{    
    Public static String ProductLineState = 'Not Found';
    Public static void ContractLineItemInsert(Map<Id,Contract_Line_Item__c> oldMap, Map<Id,Contract_Line_Item__c> newMap)
    {
    
    List<Id> CLIIds = new List<Id>();
    map<Id,String> CLIMap = new map <Id,String>(); 
    
    List<Id> Prodid = new List<Id>();
    
    List<id> PBE = new List<Id>();
    
    for (Contract_Line_Item__c c : newMap.Values())
    {   
        if(c.Product_LU__c != null)
        {      
             CLIIds.add(c.Id);
        }
    }
    
    for (Contract_Line_Item__c Prod : newMap.Values())
    {   
        if(Prod.Product_LU__c != null)
        {      
             Prodid.add(Prod.Product_LU__c);
        }
    }
    
    List<Contract_Line_Item__c> ContractLinesToUpdate = new List<Contract_Line_Item__c>();
    List<Contract_Line_Item__c> ContractLinesToUpdate1 = new List<Contract_Line_Item__c>();
    
    for(PriceBookEntry pbe1 :[Select id, UnitPrice, Pricebook2Id from PriceBookEntry where (Pricebook2id = '01s3000000001JS') and Product2id in :Prodid])
    
    {
                 
           
           for (Contract_Line_Item__c cli :[Select id, List_Price__c from Contract_Line_Item__c where id in :CLIIds])
           {
             ProductLineState = 'Not Found';
             if(pbe1.Pricebook2Id == '01s3000000001JS' && ProductLineState != 'Found' && pbe1.UnitPrice != null)
             {  
           
                cli.List_Price__c = pbe1.UnitPrice;
                ContractLinesToUpdate.add(cli);
                ProductLineState = 'Found'; 
                update(ContractLinesToUpdate);
             }
             }
    for(PriceBookEntry pbe2 :[Select id, UnitPrice, Pricebook2Id from PriceBookEntry where (Pricebook2id = '01s3000000004T2AAI') and Product2id in :Prodid])
    
    {
    for (Contract_Line_Item__c cli :[Select id, List_Price__c from Contract_Line_Item__c where List_Price__c = null and id in :CLIIds])
           {
             if(pbe2.Pricebook2Id == '01s3000000004T2AAI' && cli.List_Price__c == null &&  pbe1.UnitPrice != null)
                  {
                    cli.List_Price__c = pbe2.UnitPrice;
                    ContractLinesToUpdate.add(cli);
                    ProductLineState = 'Found'; 
                     update(ContractLinesToUpdate);
            }      }                 
           }  }
           
         }
        }

 Thank you,

Steve Laycock

JPClark3JPClark3

Steve,

You may just want to delete this code and start over. The way it is written, is very difficult to read, no comments throughout the code, so it is difficult to guess what you're attempting to do.

 

Some things to keep in mind while you rewrite the code:

You have two small for loops at the beginning that collect the exact same information.

Then your main for loops are hitting the same table, not different ones as your description specifies.

It appears that this may be called from a trigger, but that is just an assumption.

You're curly braces are all over the place.

There are if statements that check the value of variables set in the statement immediately before them.

You are also adding items to an list and performing an update of that list inside a loop. That's going to update those items multiple times.

 

When requesting help, you should show cleaner code with comments. You'll recieve much more help that way.

 

I hope this points you in the right direction.

RelaxItsJustCodeRelaxItsJustCode

JP, Sorry but you couldn't be more wrong.

 

That wasn't helpful and you don't seem to understand the code but yet you commented on it.

 

Most of what you said minus the lack of comments was totally incorrect.

 

Thank you for.... trying. 

 

Steve

 

JPClark3JPClark3

Then, explain the difference of these two loops:

    for (Contract_Line_Item__c c : newMap.Values())
    {   
        if(c.Product_LU__c != null)
        {      
             CLIIds.add(c.Id);
        }
    }
    
    for (Contract_Line_Item__c Prod : newMap.Values())
    {   
        if(Prod.Product_LU__c != null)
        {      
             Prodid.add(Prod.Product_LU__c);
        }
    }

 They are looping through the exact same set of values, checking for the exact same field to not be null. You could just perform the loop once, and get the ID and the Product_LU__c in the same loop.

 

 

In this statement, ProductLineState will never be equal to 'Found', there is no use to test for it. You're setting it to 'Not Found'. Additionally, you've already queried for PriceBookEntry where (Pricebook2id = '01s3000000001JS')

ProductLineState = 'Not Found';
if(pbe1.Pricebook2Id == '01s3000000001JS' && ProductLineState != 'Found' && pbe1.UnitPrice != null)

 

And finally, you have an update statement inside of the for loop. Everytime you add a Contract_Line_Item__c to the list, you perform an update on the whole list.

for (Contract_Line_Item__c cli :[Select id, List_Price__c from Contract_Line_Item__c where id in :CLIIds])
{
   ProductLineState = 'Not Found';
   if(pbe1.Pricebook2Id == '01s3000000001JS' && ProductLineState != 'Found' && pbe1.UnitPrice != null)
   {  
      cli.List_Price__c = pbe1.UnitPrice;
      ContractLinesToUpdate.add(cli);
      ProductLineState = 'Found'; 
      update(ContractLinesToUpdate);
   }
}

 

Your the one that asked for help becuase your code in sloppy. I was just trying to point out how you could clean it up.

 

 

RelaxItsJustCodeRelaxItsJustCode

The two loops.

 

1 is for collecting Contract Line Item Ids.

 

2 is for collection Product ids on the contract line items.

 

If the found portion of the code was working for me then I wouldn't of posted anything.

 

Lastly, updating the line with in the loop works just fine.

 

Also, Please don't try to help me in the future and you certainly need an attitude adjustment.

 

Thank you,

Steve Laycock

 

RelaxItsJustCodeRelaxItsJustCode
Oh and btw, "Your the one that asked for help becuase your code in sloppy." - No asking questions on sites like this is just being resourceful, second if your going to call someone's code "sloppy", it might do you well to spell check, as well as use the proper words.... "in sloppy" really! "becuase" really! Just let it go man.