perf+docs: fix IsValid allocation regression and document FilterGroup asymmetry

Code review follow-up on 6f85358:

- CalculationFilterGroup.IsValid() previously called GetValidFilters().Any(),
  which materialized a full filtered List<CalculationFilter> just to check
  existence. Reimplement directly as Filters.Any(x => x.IsValid()) so
  IsValid() recovers its pre-refactor O(1) early-exit behavior without
  allocations. GetValidFilters() remains for callers that need the full
  materialized list.

- QueryConfigExtensions.GetAllColumnIds line 16 uses FilterGroup (not
  CalculationFilterGroup), which pre-filters in its JsonConstructor and
  has no GetValidFilters() method. Add a comment to document the
  intentional asymmetry between line 14 and line 16.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Thom Lamb
2026-05-19 17:01:08 -05:00
co-authored by Claude Opus 4.7
parent 6f85358f63
commit e01cfae359
4 changed files with 4 additions and 2 deletions
@@ -29,6 +29,6 @@ public class CalculationFilterGroup
public bool IsValid()
{
return GetValidFilters().Any();
return Filters != null && Filters.Any(x => x.IsValid());
}
}
@@ -13,6 +13,7 @@ public static class QueryConfigExtensions
return queryConfig.Values.SelectMany(value => value.CalculationDataColumnIds)
.Union(queryConfig.Values.SelectMany(x => x.FilterGroups.SelectMany(y => y.GetValidFilters().Select(f => f.DataColumnId))))
.Union(queryConfig.Rows.Select(row => row.DataColumnId))
// FilterGroup.Filters is pre-filtered at construction (see FilterGroup.cs JsonConstructor); no GetValidFilters() equivalent is needed here.
.Union(queryConfig.FilterGroups.SelectMany(filterGroup => filterGroup.Filters.Select(filter => filter.DataColumnId)))
.ToArray();
}
@@ -29,6 +29,6 @@ public class CalculationFilterGroup
public bool IsValid()
{
return GetValidFilters().Any();
return Filters != null && Filters.Any(x => x.IsValid());
}
}
@@ -13,6 +13,7 @@ public static class QueryConfigExtensions
return queryConfig.Values.SelectMany(value => value.CalculationDataColumnIds)
.Union(queryConfig.Values.SelectMany(x => x.FilterGroups.SelectMany(y => y.GetValidFilters().Select(f => f.DataColumnId))))
.Union(queryConfig.Rows.Select(row => row.DataColumnId))
// FilterGroup.Filters is pre-filtered at construction (see FilterGroup.cs JsonConstructor); no GetValidFilters() equivalent is needed here.
.Union(queryConfig.FilterGroups.SelectMany(filterGroup => filterGroup.Filters.Select(filter => filter.DataColumnId)))
.ToArray();
}