Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change MaskPattern.Patterns to a list #531

Merged
merged 4 commits into from
May 22, 2024

Conversation

Shane32
Copy link
Contributor

@Shane32 Shane32 commented May 19, 2024

This code changes MaskPattern.Patterns to a list instead of a dictionary to hopefully make some code easier to understand.

Existing code

The index is stored zero-based within the QR code, and I was confused by this code here:

var formatStr = GetFormatString(eccLevel, pattern.Key - 1);

followed by

selectedPattern = pattern.Key;

and

qrCode.ModuleMatrix[y][x] ^= MaskPattern.Patterns[selectedPattern.Value](x, y);
qrCode.ModuleMatrix[x][y] ^= MaskPattern.Patterns[selectedPattern.Value](y, x);

which is actually correct because of

return selectedPattern.Value - 1;

So since the dictionary is unnecessary, I converted it to a list. It should be faster too but I didn't check.

New code

Now we start with this:

for (var maskPattern = 0; maskPattern < 8; maskPattern++)
{
var patternFunc = MaskPattern.Patterns[maskPattern];

and uses are simple:

var formatStr = GetFormatString(eccLevel, maskPattern);

selectedPattern = maskPattern;

return selectedPattern.Value;

@Shane32
Copy link
Contributor Author

Shane32 commented May 22, 2024

I'll need to fix conflicts here

QRCoder/QRCodeGenerator.ModulePlacer.cs Outdated Show resolved Hide resolved
QRCoder/QRCodeGenerator.ModulePlacer.cs Outdated Show resolved Hide resolved
@codebude
Copy link
Owner

Can you also have a look on the merge conflicts?
image

@Shane32 Shane32 requested a review from codebude May 22, 2024 20:37
@codebude codebude merged commit e3664d1 into codebude:master May 22, 2024
3 checks passed
@Shane32 Shane32 deleted the maskpattern_list branch May 22, 2024 22:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants