Skip to main content
added 6 characters in body
Source Link
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(oliitem.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(oliitem); // 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(oliitem.OpportunityId))
        byParent.put(oliitem.OpportunityId, new List<OpportunityLineItem>());
    List<OpportunityLineItem> lineItems = byParent.get(oliitem.OpportunityId);
    // now you've put something in the map
    // you always know this list will be non-null

    lineItems.add(oliitem); // 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)
);

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

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(item.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(item); // 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(item.OpportunityId))
        byParent.put(item.OpportunityId, new List<OpportunityLineItem>());
    List<OpportunityLineItem> lineItems = byParent.get(item.OpportunityId);
    // now you've put something in the map
    // you always know this list will be non-null

    lineItems.add(item); // 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)
);
added 1467 characters in body
Source Link
Adrian Larson
  • 151.3k
  • 38
  • 247
  • 431

You never actually put a listYou're doing two things at once: filtering and grouping. The grouping is where you're going astray.

Here's what you are doing in for a given key. You also should map by OpportunityId, not Idmore expanded format. A method to do the grouping would look like:

publicMap<Id, staticList<OpportunityLineItem>> byParent = new Map<Id, List<Opportunity>>List<OpportunityLineItem>>();
for groupByParent(OpportunityLineItem item : trigger.new)
{
    if (/*condition*/)
    {
        List<OpportunityLineItem> itemslineItems = 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>> groupedbyParent = new Map<Id, List<OpportunityLineItem>>();
    for (OpportunityLineItem item : itemstrigger.new)
    {
        if (!groupedbyParent.containsKeycontains(itemoli.OpportunityId))
            groupedbyParent.put(itemoli.OpportunityId, new List<OpportunityLineItem>());
     List<OpportunityLineItem> lineItems = groupedbyParent.get(itemoli.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, returnList<OpportunityLineItem>> grouped;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)
);

You never actually put a list in for a given key. You also should map by OpportunityId, not Id. A method to do the grouping would look like:

public static Map<Id, List<Opportunity>> groupByParent(List<OpportunityLineItem> items)
    Map<Id, List<OpportunityLineItem>> grouped = new Map<Id, List<OpportunityLineItem>>();
    for (OpportunityLineItem item : items)
    {
        if (!grouped.containsKey(item.OpportunityId))
            grouped.put(item.OpportunityId, new List<OpportunityLineItem>());
        grouped.get(item.OpportunityId).add(item);
    }
    return grouped;
}

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)
);
Source Link
Adrian Larson
  • 151.3k
  • 38
  • 247
  • 431

You never actually put a list in for a given key. You also should map by OpportunityId, not Id. A method to do the grouping would look like:

public static Map<Id, List<Opportunity>> groupByParent(List<OpportunityLineItem> items)
    Map<Id, List<OpportunityLineItem>> grouped = new Map<Id, List<OpportunityLineItem>>();
    for (OpportunityLineItem item : items)
    {
        if (!grouped.containsKey(item.OpportunityId))
            grouped.put(item.OpportunityId, new List<OpportunityLineItem>());
        grouped.get(item.OpportunityId).add(item);
    }
    return grouped;
}