Remove legacy OneToFourBitPattern function from Code93 decoder
#972
Conversation
core/src/oned/ODCode93Patterns.cpp
Outdated
|
|
||
| namespace ZXing::OneD::Code93 { | ||
|
|
||
| const std::array<std::array<int, 6>, 48> CODE_PATTERNS = { { |
There was a problem hiding this comment.
I'd prefer to see this in the header and making it constexpr.
| // quiet zone is half the width of a character symbol | ||
| constexpr float QUIET_ZONE_SCALE = 0.5f; | ||
|
|
||
| //TODO: make this a constexpr variable initialization | ||
| static auto E2E_PATTERNS = [ ] { |
There was a problem hiding this comment.
Would you be able to implement this TODO (see also comment above)?
| @@ -62,46 +61,6 @@ static bool DecodeDigit(const PatternView& view, std::string& txt, int* lgPatter | |||
| AppendBit(*lgPattern, bestMatch >= 10); | |||
|
|
|||
| return true; | |||
| #else | |||
There was a problem hiding this comment.
Please leave this disabled code alone. It might be useful as documentation in the future.
|
Thank you for the feedback. I’ve applied all your suggestions except for making E2E_PATTERNS a constexpr. The reason is that ToInt is currently used in the computation, which makes it difficult to convert directly under the current structure. While it's possible to work around this by rewriting the logic differently, or by manually declaring all patterns as done in CODE_PATTERNS, I wasn't sure which approach you'd prefer. Also, I’m currently tied up with other tasks, so I couldn’t find the time to implement either option at the moment. Apologies for postponing that part, and please feel free to share any further thoughts on the current changes. |
core/src/oned/ODCode128Patterns.cpp
Outdated
| @@ -8,7 +8,7 @@ | |||
|
|
|||
| namespace ZXing::OneD::Code128 { | |||
|
|
|||
| const std::array<std::array<int, 6>, 107> CODE_PATTERNS = { { | |||
| constexpr std::array<std::array<int, 6>, 107> CODE_PATTERNS = { { | |||
There was a problem hiding this comment.
Please leave the Code128 part as is for now.
core/src/oned/ODCode93Patterns.cpp
Outdated
|
|
||
| namespace ZXing::OneD::Code93 { | ||
|
|
||
| constexpr std::array<std::array<int, 6>, 48> CODE_PATTERNS = { { |
There was a problem hiding this comment.
Here I meant move the definition of the variable into the header and drop the cpp-file completely. I'm not sure that adding constexpr at this location changes anything at all.
|
Sorry about that. I accidentally clicked it.. :'( I’ll push the updated changes very soon. |
|
I’ve made the requested changes. I’m truly sorry if my lack of experience caused any unnecessary trouble or wasted your valuable time. |
This reverts commit b8cd1e2.
This PR removes the
OneToFourBitPatternfunction, which was part of the legacy Code93 decoding logic.Code93 now consistently uses
NormalizedE2EPattern, instead ofOneToFourBitPattern.Removing this helps simplify the code and reduce technical debt.
Key changes:
OneToFourBitPatternwith the standardizedNormalizedE2EPatternapproach, already used across other symbologies.ODCode93Patterns.cpp: Contains theCODE_PATTERNSdefinition for Code93.ODCode93Patterns.h: Corresponding header for pattern definitions.Modified files:
ODCode93Reader.cpp: RemovedOneToFourBitPattern, updated decoding logic to useNormalizedE2EPattern.ODMultiUPCEANReader.cpp: Removed residual mentions ofOneToFourBitPatternfrom an unused/dead code path inDecodeDigit. This was done for cleanup and consistency.ODRowReader.h: RemovedOneToFourBitPattern, which is no more needed.core/CMakeLists.txt: Added the newODCode93Patternsfiles to the build.Please review the changes and let me know if further adjustments are needed.
Thank you.