Starting November 20, the site will be set to read-only. On December 4, 2023,
forum discussions will move to the Trailblazer Community.
ShowAll Questionssorted byDate Posted
sachin joshi 14

# Trigger is not working

Hi All,
I have wrote a trigger on opportunity to update account fields, I want to populate account field(MaxoppName__c) with opportunity name which has maximum Amount below is my code, but it is not working
```trigger SumAmount_Count_MaxOpportunity on Opportunity (after insert,after update) {
List<Account> Ac = new List<Account>();
Map<String, Decimal> M = new Map<String, Decimal>();
set<Id> accId = new set<Id>();
for(Opportunity objOpp: trigger.new){
}
Decimal Sum;
Integer Count;
for(Account Acc : [SELECT Id,Name,(SELECT Id,Name,Amount FROM Opportunities)
FROM Account WHERE Id IN : accId]){
Sum=0;
Count=0;
for(Opportunity Opp: Acc.Opportunities){
M.put(opp.Name, opp.Amount);
Sum += Opp.Amount ;
Count++;

List<Decimal> l = m.values();
for(integer i=0; i<l.size(); i++){
Decimal Max;
if (Max < l[i]){
Max = l[i];
opportunity op = [select id, name from opportunity where Amount =: Max];

acc.MaxoppName__c = op.name;
}
}
}
Acc.Amount__c = Sum;
Acc.OpportunityCount__c = Count;
}
update Ac;
}```
other fields are updating but not MaxoppName__c Please give suggesions.

Best Answer chosen by sachin joshi 14
Le Nguyen
Hi,

1. you put the Decimal Max; inside the loop.  Therefore, with every value of list l, the Max is null.
You can fix it to move it outside the calculation loop.
```List<Decimal> l = m.values();
Decimal Max = 0;
for(integer i=0; i<l.size(); i++){

if (Max < l[i]){
Max = l[i];
opportunity op = [select id, name from opportunity where Amount =: Max];

acc.MaxoppName__c = op.name;
}
}
}```

2.  This is not a good way to do it.  You put a query in side every step of the Account's Amound Calculation.  If you do a mass update or insert, this will throw you an too many SOQL error.

I suggest you try this codes below:
```for(Account Acc : [SELECT Id,Name,(SELECT Id,Name,Amount FROM Opportunities)
FROM Account WHERE Id IN : accId]){
Sum=0;
Count=0;
for(Opportunity Opp: Acc.Opportunities){
M.put(opp.Name, opp.Amount);
Sum += Opp.Amount ;
Count++;
}
List<Decimal> l = m.values();
string MaxName = '';
if(l != null && l.size() > 0){
l.sort();
Decimal Max = l[l.size() -1];

for(string s : M.keyset()){
Decimal Mvalue = M.get(s);
if(Mvalue == Max){
MaxName = s;
break;
}
}
}
Acc.MaxoppName__c = MaxName;
Acc.Amount__c = Sum;
Acc.OpportunityCount__c = Count;
}
update Ac;```

3. You can do rollup summary for Opportunity Amount Sum and Opportunity Count.

Le

Le Nguyen
Hi,

1. you put the Decimal Max; inside the loop.  Therefore, with every value of list l, the Max is null.
You can fix it to move it outside the calculation loop.
```List<Decimal> l = m.values();
Decimal Max = 0;
for(integer i=0; i<l.size(); i++){

if (Max < l[i]){
Max = l[i];
opportunity op = [select id, name from opportunity where Amount =: Max];

acc.MaxoppName__c = op.name;
}
}
}```

2.  This is not a good way to do it.  You put a query in side every step of the Account's Amound Calculation.  If you do a mass update or insert, this will throw you an too many SOQL error.

I suggest you try this codes below:
```for(Account Acc : [SELECT Id,Name,(SELECT Id,Name,Amount FROM Opportunities)
FROM Account WHERE Id IN : accId]){
Sum=0;
Count=0;
for(Opportunity Opp: Acc.Opportunities){
M.put(opp.Name, opp.Amount);
Sum += Opp.Amount ;
Count++;
}
List<Decimal> l = m.values();
string MaxName = '';
if(l != null && l.size() > 0){
l.sort();
Decimal Max = l[l.size() -1];

for(string s : M.keyset()){
Decimal Mvalue = M.get(s);
if(Mvalue == Max){
MaxName = s;
break;
}
}
}
Acc.MaxoppName__c = MaxName;
Acc.Amount__c = Sum;
Acc.OpportunityCount__c = Count;