Roll up Summary via trigger for lookup relationship

Question

I am trying to get this trigger to work but it is giving me an error and I am not able to figure out why. The error I am getting is "attempt to de-reference a null object)

Why trigger? We have a Destinations object which is linked to Supplier products object via a lookup relationship. There are two types of supplier products(marketplace and curated) We want to count the number of live Supplier products and put them in a separate fields(live counts for each type).

I have gotten so far to actually count the number of live products and it is giving me an error. Can any one help? I am posting the code below

Trigger code :

trigger NoOfLiveProducts on Supplier_Product__c (after insert, after Delete, after update, after undelete) {


    set<string> destinationids = new set<string>();

    if(trigger.isInsert) {
        for(Supplier_product__c products : Trigger.new) {
            destinationids.add(products.Destination__c);
        }
    } else if(trigger.isDelete) {
        for(Supplier_product__c products : Trigger.old) {
            destinationids.add(products.Destination__c);
        }
    }else if(trigger.isUnDelete) {
        for(Supplier_product__c products : Trigger.new){
            destinationids.add(products.Destination__c);
        }

    } else if(trigger.isUpdate) {
        for(Supplier_product__c products : Trigger.new) {
            destinationids.add(products.Destination__c);
        }
    }

    if(destinationids.size() > 0){
        Productcounter pc = new Productcounter();
        pc.updateproductCount(destinationids);
    }



}

Class code :

public class Productcounter {

    public void updateproductcount(set<string> productIDs) {

         if(productIDs == null){   
            return;
        }

        if(productIDs.size() == 0){
            return;
        }

         list<Supplier_Product__c> ProductList = new list<Supplier_Product__c>();
        ProductList = [Select ID,Destination__c
                    From Supplier_Product__c
                    Where Destination__c = :productIDs];

        integer totalCount;
        map<string, integer> DestinationIDToProductCountMap = new map<string, integer>();       
        for(Supplier_product__c ProductItem : ProductList){
            if(DestinationIDToProductCountMap.containsKey(ProductItem.Destination__c)){
                totalCount = DestinationIDToProductCountMap.get(ProductItem.Destination__c);
            }else{
                totalCount = 0;
            }
            totalCount += 1;
            DestinationIDToProductCountMap.put(ProductItem.Destination__c, totalCount);
        }

        list<Destination__c> DestinationUpdateList = new list<Destination__c>();
        Destination__c DestinationTemp;
        for(string ProductID : DestinationIDToProductCountMap.keySet()){
            DestinationTemp.ID = ProductID;
            DestinationTemp.No_of_Live_Curated_Products__c = DestinationIDToProductCountMap.get(ProductID);
            DestinationUpdateList.add(DestinationTemp);
        }
        if(DestinationUpdateList.size() > 0){
            update DestinationUpdateList;
        }

         }

         }

Answer 1

Your code returns a null pointer exception because you never assigned a value to DestinationTemp before attempting to reference it (the ID field).

You'd want to do this:

for(...
    DestinationTemp = new Destination__c();
    DestinationTemp.Id = ProductId;
    ...

Also, some tips for the future:

If your Destination__c can change, you should use a simpler Trigger.new/Trigger.old approach:

if(Trigger.old != null) {
    for(Supplier_Product__c record: Trigger.old) {
        destinationIds.add(record.Destination__c);
    }
}
if(Trigger.new != null) {
    for(Supplier_Product__c record: Trigger.new) {
        destinationIds.add(record.Destination__c);
    }
}

This handles the situation where a destination might change because of a field edit. Even if you don't allow that change, this code is still more efficient to cover than your original code.

Do not check for nulls that cannot happen. Your function is called with a value that is never null, but you check anyways:

if(productIDs == null){   
        return;
    }

Further, your code checks to see if that list is empty... TWICE!

if(destinationids.size() > 0){ 

if(productIDs.size() == 0){

Also, assuming Destination__c is a required field, size() will never, ever be empty. I guarantee it. You can remove all those checks and save time, increase code coverage, etc.

Next, you could be using an aggregate result query:

AggregateResult[] sums = [SELECT Count(Id), Destination__c FROM Supplier_Product__c WHERE Destination__c IN :productIds GROUP BY Destination__c];

All you need to do then is make a simple Map to update the destinations:

Map<Id, Destination__c> destinations = new Map<Id, Destination__c>();
for(Id productId: productIds) {
    destinations.put(productId, new Destination__c(Id=productId, No_of_Live_Curated_Products__c=0.0));
}
for(AggregateResult result: [SELECT Count(Id) total, Destination__c Id FROM Supplier_Product__c WHERE Destination__c IN :productIds GROUP BY Destination__c]) {
    destinations.get((Id)result.get('Id')).No_of_Live_Curated_Products__c = (Decimal)result.get('total');
}
update destination.values();

Also, as ProductIds is never, ever empty, the DML statement will also never be empty...

Next, don't make instances when you could be using static functions:

ProductCounter.updateProductCount(productIds);

...

public static void updateProductCount(Set<Id> productIds) { ...

Finally, you really should be using an ID type when the type is ID, not String. This makes it clear to readers that the values are obviously ID values, not some other value like, say, the name of a record or something.

Set<Id> productIds = new Set<Id>();
...

---

trigger NoOfLiveProducts on Supplier_Product__c (after insert, after Delete, after update, after undelete) {
    set<id> destinationids = new set<id>();
    if(trigger.new != null) {
        for(Supplier_product__c products : Trigger.new) {
            destinationids.add(products.Destination__c);
        }
    }
    if(trigger.old != null) {
        for(Supplier_product__c products : Trigger.old) {
            destinationids.add(products.Destination__c);
        }
    }
    ProductCounter.updateproductCount(destinationids);
}

---

public class Productcounter {
    public void updateproductcount(set<id> productIDs) {
        Map<Id, Destination__c> destinations = new Map<Id, Destination__c>();
        for(Id productId: productIds) {
            destinations.put(productId, new Destination__c(Id=productId, No_of_Live_Curated_Products__c=0.0));
        }
        for(AggregateResult result: [SELECT Count(Id) total, Destination__c Id FROM Supplier_Product__c WHERE Destination__c IN :productIds GROUP BY Destination__c]) {
            destinations.get((Id)result.get('Id')).No_of_Live_Curated_Products__c = (Decimal)result.get('total');
        }
        update destination.values();
    }
}