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