Skip to main content
2 of 3
added 1467 characters in body
Adrian Larson
  • 151.3k
  • 38
  • 247
  • 431

You're doing two things at once: filtering and grouping. The grouping is where you're going astray.

Here's what you are doing in a more expanded format.

Map<Id, List<OpportunityLineItem>> byParent = new Map<Id, List<OpportunityLineItem>>();
for (OpportunityLineItem item : trigger.new)
{
    if (/*condition*/)
    {
        List<OpportunityLineItem> lineItems = byParent.get(oli.Id);
        // lineItems is null because you never put anything in the map
        // so of course you can't add anything to it yet

        lineItems.add(oli); // will throw NPE
    }
}

Here's how the grouping part should look.

Map<Id, List<OpportunityLineItem>> byParent = new Map<Id, List<OpportunityLineItem>>();
for (OpportunityLineItem item : trigger.new)
{
    if (!byParent.contains(oli.OpportunityId))
        byParent.put(oli.OpportunityId, new List<OpportunityLineItem>());
    List<OpportunityLineItem> lineItems = byParent.get(oli.OpportunityId);
    // now you've put something in the map
    // you always know this list will be non-null

    lineItems.add(oli); // will not throw NPE
}

The filtering would be best done separately:

List<OpportunityLineItem> dateRangeChanged = new List<OpportunityLineItem>();
for (OpportunityLineItem item : trigger.new)
{
    if (/*condition*/) dateRangeChanged.add(item);
}

You can make this functionality more reusable by moving it into separate, static methods.

public static void List<OpportunityLineItem> hasDateRangeChanged
    (List<OpportunityLineItem> newRecords, Map<Id, OpportunityLineItem> oldMap)
{
    // implementation
}
public static void Map<Id, List<OpportunityLineItem>> groupByParent
    (List<OpportunityLineItem> items)
{
    // implementation
}

Then in your trigger, you can just do:

Map<Id, List<OpportunityLineItem>> byParent = OliService.groupByParent(
    OliService.hasDateRangeChanged(trigger.new, trigger.oldMap)
);
Adrian Larson
  • 151.3k
  • 38
  • 247
  • 431