Closed dapryor closed 4 years ago
Merging #19 into master will not change coverage. The diff coverage is
100%
.
@@ Coverage Diff @@
## master #19 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 3 3
Lines 58 60 +2
=====================================
+ Hits 58 60 +2
Impacted Files | Coverage Δ | |
---|---|---|
matcher.go | 100% <100%> (ø) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 984a299...3105891. Read the comment docs.
I have an idea about this! Let's write some benchmark to compare regexp version and @Srivats1991 implementation. Then we can look for ways to improve.
I really wonder about efficiency of regexp implementations.
(#19)Benchmark_MatchDaysOfWeek-8 119097 10099 ns/op 908 B/op 23 allocs/op (#3)Benchmark_MatchDaysOfWeek2-8 227019 4626 ns/op 4201 B/op 33 allocs/op
#3 | #19 | |
---|---|---|
µs | 4.6 | 10.1 |
KB\op | 4.201 | 0.908 |
Allocs/op | 33 | 23 |
So Regexp did slow it down but has a better memory footprint. Depends on what means most to you.
Regexp is probably overkill, but was just utilizing the functions already in place.
Thanks for the comparison.
I guess that will be better to go with #3. We can extract the match in order functionality from @Srivats1991 implementation to use it further in another feature implementations and we would have a fun challenge to make it more efficient to boost the performance of every matcher uses it.
Also I want to see some implementation over regexp expressions in our codebase. Because regexp implementations are what they are. You can't change a lot about them. If we write our implementations for appropriate ones, that will be more fun and more open for improvement.
This is my take on Match Days of Week. If you like this, merge it but merge Srivats1991 first to give him credit for his work if he is doing Hacktoberfest. If you don't like this, deny it :)
Also, if you need a visual representation of my regex: