Do write your "if"s instead of using null coalescing set.
Why?
Because when running your tests, code coverage will only pass on a line, so your one-liner will be passed on, whether the "default value" was set or not.
By keeping the "if", the code is less dense, it's true, but at least your code coverage will help you know whether this specific case is covered or not by your test suite.
I'm annoyed by that and some people hate it when I "add code" like that, but trust me, it's better ☺️
@dgoosens @pierstoval I think that still gets treated as one line. In the report, you might see the first line as green and the second as white/clear, though, instead of red/pink.
@pierstoval Does branching coverage catch the conditional?
@pierstoval I guess I could tag @derickr or @sebastian because either one probably knows. 🙂
Also, I think I called it the wrong thing. I think it’s normally referred to as “path coverage.”
@ramsey Both "Branch Coverage" and "Path Coverage" exist, see https://phpunit.expert/articles/path-coverage-or-mutation-testing.html?ref=mastodon for details.
@sebastian Thanks! I didn’t realize they were two separate things.
Can one of them show if you haven’t tested all conditions of `??=` ?
@ramsey I'm also sure that @derickr and @sebastian know about it 😁
Still, I don't think it's even possible to break coverage into partial statements like that, it would need a much harder check at runtime, implying current context from the call stack, etc., which I think coverage tools can't actually cover.
It also implies changing how current code coverage is stored and displayed, and even difftools sometimes struggle with single-line diffs...
But maybe it's possible, I don't know 😅
@pierstoval At work, we have some specialized cyclomatic complexity code sniffs that factor in symbols like `??=` into the complexity score of a function.
@ramsey Specialized sniffs ? Why not use the `Generic.Metrics.CyclomaticComplexity` sniff ? It already takes null coalesce into account.
No doubt there may be (other) things which could be improved, but I'll happily review a PR to get that sorted, if so submitted.
/cc @pierstoval
@ramsey Just curious to hear why, not meant as criticism.
@jrf_nl Potentially two reasons:
1. The sniff we have is very, very old and maybe predates the Generic one?
2. NIH syndrome
This code base is over 20 years old, and along the way it has had thousands of different developers working in it, so who really knows?
@ramsey Understood. If they'd be interested, I could take a look and suggest some optimizations.
P.S.: just checked - the Generic sniff was introduced in 2007 ;-)
