refactor: replace switch statements with match expressions#2524
Merged
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 1.x #2524 +/- ##
============================================
+ Coverage 86.27% 86.34% +0.07%
+ Complexity 22924 22890 -34
============================================
Files 1739 1739
Lines 70415 70380 -35
============================================
+ Hits 60753 60773 +20
+ Misses 9662 9607 -55 🚀 New features to boost your workflow:
|
Converts 9 hand-written switch statements across core ETL, parquet lib and http/parquet/seal adapters to match expressions: strict comparison, no break fall-through, single-expression dispatch (net -100 lines). - explicit default arms preserve original no-default fall-through behavior (match throws UnhandledMatchError otherwise) - SchemaConverters get a small unwrapOptional() helper because repeated accessor calls inside one match arm defeat static-analysis narrowing - Thrift protocol code, SnappyDecompressor hot loop, PageReader, FlowTelemetryBundle and PlainValuesPacker stay as switch (bodies not expressible as match arms without restructuring) Verified: 4083 tests green, mago clean; 1M-row pipeline outputs are byte-identical (CSV and parquet md5), timings within noise.
b202778 to
30d7a67
Compare
Member
|
I merged it mostly due to better DX, the perf improvement is hard to verify here |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces
switchstatements withmatchexpressions.Changes
Flow\ETL\Dataset\Memory\Unit::fromString()— dispatch and the default digit-or-throw branch as onematch(throw-as-expression),Flow\ETL\Formatter\ASCII\ASCIIValue::mb_str_pad()— padding lengths hoisted above thematch;default => $resultreproduces the original no-default fall-through,Flow\ETL\Adapter\Http\RequestEntriesFactory— body-entry selection as amatchassignment,Flow\ETL\Adapter\Parquet\SchemaConverter::flowToParquet()— 14-armmatch; the$nullable ? OPTIONAL : REQUIREDternary repeated in every case is hoisted into$repetition;ListType/MapTypearms unwrap optionals through a smallunwrapOptional()helper (repeated accessor calls defeat static-analysis narrowing inside a single arm expression),Flow\ETL\Adapter\Seal\SchemaConverter::flowToSealField()— same treatment,Flow\Parquet\WriterandFlow\Parquet\Writer\ColumnChunkBuilderFactory— compression/encoding validation asmatchwithcondition ? throw : nullarms,Flow\Parquet\Writer\PageBuilder\DictionaryBuilder— previously a hybridswitcharound nestedmatches, now a single nestedmatch,Flow\Parquet\Dremel\Validator\ColumnDataValidator— nested validationmatches; inner switches had nodefault, so the arms get an explicitdefault => nullto preserve the silent pass-through, andcase null:becomes a strictnullarm.Left as
switchon purpose:ThriftModel/*andCompactProtocol(Thrift protocol field-id dispatch with multi-statement bodies / by-ref temporaries),SnappyDecompressor(hot loop with multi-variable assignments per case),PageReader,FlowTelemetryBundle(multi-statement DI configuration cases) andPlainValuesPacker(per-case@varassertions that static analysis relies on). None of these can become amatchwithout restructuring beyond a mechanical conversion.No public API changes. Every converted switch was checked for loose-vs-strict comparison hazards (subjects are enums,
::classstrings orstrtoupper()results) and for missing-defaultfall-throughs (matchthrowsUnhandledMatchErrorwhereswitchfalls through — explicitdefaultarms preserve the original behavior).Verification & Benchmarks
Standard pipeline on 1M rows (
from_csv→batchSize(1000)→withEntry(concat)→filter(endsWith)→ write), PHP 8.3 + opcache, fresh process per run, medians of 3:1.xPeak memory unchanged (16/18 MB). Output files are byte-identical before and after the change (matching md5 for the 91 MB CSV and the 24.2 MB parquet output — the parquet variant deliberately routes through the converted
SchemaConverter,DictionaryBuilderand per-valueColumnDataValidator). As expected for switch→match, this is a readability/strictness refactor, not a performance change — the benchmark is regression evidence.Resolves: no related roadmap item — standalone modernization refactor
Change Log
Added
Fixed
Changed
Removed
Deprecated
Security