0

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;
        }

         }

         }
  • 1
    for future reference, the free package 'Declarative Lookup Rollup Summary (DLRS)' by Andrew Fawcett can solve these kinds of problems in a point-and-click manner – cropredy Jun 30 '15 at 0:15
  • I tried that and it doesn't work because it has to count the number of records and it is not rolling up a particular field. I have run into another problem here. Can you help? – Dev Jul 6 '15 at 17:35
  • did the @sfdcfox solution not work for you? He's invariably accurate – cropredy Jul 6 '15 at 17:55
  • That worked but I need to segment the number of live products based on a picklist field called "type"(curated and marketplace) on the object "Supplier Products". At the moment, I am counting all the products(curated and marketplace) and throwing it into one. – Dev Jul 6 '15 at 18:04
  • Here is my new code that I used, I have posted it in chunks trigger NoOfLiveProducts on Supplier_Product__c (after insert, after Delete, after update) { Set<id> destinationIds = new Set<id>(); List<Destination__c> destinationsToUpdate = new List<Destination__c>(); for (Supplier_Product__c item : Trigger.new) destinationIds.add(item.Destination__c); if (Trigger.isUpdate || Trigger.isDelete) { for (Supplier_Product__c item : Trigger.old) destinationIds.add(item.Destination__c); } System.debug('>>>>destinationIds>>>>'+destinationIds); – Dev Jul 6 '15 at 18:06
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();
    }
}
|improve this answer|||||
  • I am getting the error now stating that there is no field called "total" in the query aggregate result query. Any ideas? – Dev Jun 29 '15 at 23:01
  • @Dev I actually forgot that it needed to be Count(Id) instead of just Count(). The message you'd get is if you didn't alias the Count(Id) function... I've just made an edit, so take a peek. – sfdcfox Jun 29 '15 at 23:06
  • I used a different code than you suggested. Can you please look at comments above and help me please. I have posted the code and comments above in another comment. Please have a look if you can. thanks – Dev Jul 6 '15 at 18:13

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.