There are a number of issues with your code. So many that I don't want to go into detail with them all. First is that it would be very helpful to you to separate your initial test for data into an
If(is.afterInsert)
}
for(Opportunity o : Trigger.New)
}
forecastDuration.put(o.Id, o.Implementation_Revenue__c.monthsBetween(o.FSR_End_Date__c));
{
upOpps.addAll(trigger.new);
{
// there's nothing in trigger.old to test against, so no need in AfterInsert
If(isAfterUpdate)
}
for(Opportunity o : Trigger.New)
}
// record selection code here
{
{
Opportunity oldO = Trigger.OldMap.get(o.Id);
For you selection criteria in AfterUpdate:
// try{
// Opportunity oldO = Trigger.OldMap.get(o.Id);
You don't want to use a try-catch block in your trigger. This is cpu intensive. With separate blocks, this is not necessary. Test it and see. If fields aren't required or validated, do a null test instead.
// record selection code
for(Opportunity o : Trigger.New)
}
if(oldO.CloseDate != o.CloseDate
||
oldO.Annual_Settlement_Value__c != o.Annual_Settlement_Value__c
||
oldO.Implementation_Revenue__c != o.Implementation_Revenue__c
||
oldO.Implementation_Revenue_as_a_of_FSR__c != oldO.Implementation_Revenue_as_a_of_FSR__c
||
oldO.FSR_End_Date__c != o.FSR_End_Date__c
||
oldO.Monthly_Txn_Revenue_POS__c != o.Monthly_Txn_Revenue_POS__c
||
oldO.Annual_transaction_volume__c != o.Annual_transaction_volume__c)
{
upOpps.add(o);
forecastDuration.put(o.Id, o.Implementation_Revenue__c.monthsBetween(o.FSR_End_Date__c));
{
This is SIX OR conditions to test at the same time. It might behoove you to break these up into 6 If conditions where you add the ID's to a set. Once you have them, at the end, add all of them to a single set. Let's call it OupIds for our purposes.
You can convert OupIds to a list from trigger.newmap to get your list of opportunities and create your map for the oppIds that are in the set should you need to. I suspect that a test will show that doing it this way will prove faster, but don't have comparable data at the moment I can test with to prove one way or another. I can only guarantee that it will be faster than repeating the try-catch that you're using.
for(Opportunity o : [SELECT Id,Name,Implementation_Revenue__c,Implementation_Revenue_as_a_of_FSR__c,
Annual_Settlement_Value__c,Annual_transaction_volume__c,Monthly_Txn_Revenue_POS__c,
CurrencyIsoCode
FROM Opportunity
WHERE Id IN :insOpps])
{
You don't need to run this query. You will already have all of this info. It should be contained in trigger.new or trigger.newmap for the Ids that you've determined need to be processed.
I would run the following loop on OupIds, not on the object and pull the data from trigger.newmap.
For(Id oid:OupIds)
}
Date ic = trigger.newmap.get(oid).Implementation_Revenue__c;
Integer icm = trigger.newmap.get(oid).Implementation_Revenue__c.month();//DELETE?
//Ramp calculations
Decimal increment = trigger.newmap.get(oid).Implementation_Revenue_as_a_of_FSR__c / 100;
// You're inside of a loop, don't define each of these variables each time through!!!
// Initialize them once and only once outside the loop instead
Integer rDur = (1 / increment).setscale(0,roundingMode.DOWN).intValue();
Integer fcstDuration = forecastDuration.get(oid);
Decimal monthlySett = trigger.newmap.get(oid).Annual_Settlement_Value__c / fcstDuration;
Decimal annualTxnRev = trigger.newmap.get(oid)..Annual_transaction_volume__c*trigger.newmap.get(oid).Monthly_Txn_Revenue_POS__c;
Decimal monthlyTxnRev = annualTxnRev/fcstDuration;
//FSR calculations
Integer fsrDur = forecastDuration.get(oId) - rDur;
Decimal settLoopTotal = 0.00;
Decimal txnrevLoopTotal = 0.00;
//add Monthly Forecast records for 'ramp' months
for(Integer m = 0; m < rDur; m++){
Decimal ramp = increment*(m + 1);
Decimal monthSett = monthlySett*ramp;
Decimal monthTxnRev = monthlyTxnRev*ramp;
// ### same comments here on these variables, you're recreating them within an inner loop!!!
Monthly_Revenue_Forecast__c fm = new Monthly_Revenue_Forecast__c();
fm.Opportunity__c = oId;
fm.CurrencyIsoCode = trigger.newmap.get(oid).CurrencyIsoCode;
fm.Name = monthName.get(ic.addMonths(m).month()) +' '+ ic.addMonths(m).year();
fm.Revenue_Date__c = ic.addMonths(m + 1).toStartOfMonth() - 1;
fm.Monthly_Settlement1__c = monthSett;
fm.Transaction_Revenue__c = monthTxnRev;
settLoopTotal = settLoopTotal + monthSett;
txnrevLoopTotal = txnrevLoopTotal + monthTxnRev;
newMRFs.add(fm);
if(m == rDur - 1){
rampSett.put(oid,settLoopTotal);
rampTxnRev.put(oid,txnrevLoopTotal);
settLoopTotal = 0;
txnrevLoopTotal = 0;
}
}
I'm certain you can see where these comments would apply to the remaining portions of your code.
