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

Code rustification; where to start? #8

Open
Asha20 opened this issue Feb 14, 2023 · 2 comments
Open

Code rustification; where to start? #8

Asha20 opened this issue Feb 14, 2023 · 2 comments

Comments

@Asha20
Copy link
Contributor

Asha20 commented Feb 14, 2023

Hi! I'd love to contribute to this project. Lately I've been using zxing-cpp for a hobby project that involved scanning QR codes in a web browser and seeing a Rusty version pop up on the r/rust front page put a smile on my face. :)

I'd like to help "rustify" the code, to improve readability and get to know the codebase a bit better without making major changes. I see in your Reddit post that you have in-progress work though, so before I start opening PRs, I wanted to ask:

What are some good files in the repo to get started with; files that you aren't working on currently, which could use some prettification?

@hschimke
Copy link
Collaborator

The three main things that I would classify as “ongoing” are porting the QRCode implementation from zxing-cpp, adding reader-reset to symbologies that support it, and building a first-class bytes return (right now the way in which we handle returning bytes is all over the place, and I’d like to unify that. However, this is probably part of a bigger rework as I try to untangle the text encoder from the library and unify the result returns in general)

So, I think the modules that I would suggest avoiding are: QR and ECIStringBuilder. In my opinion the PDF417 implementation is the messiest, it just didn’t smoothly translate into rust in a lot of cases, or I didn’t know how to smoothly translate it, so there are some weird issues with nesting options and side effects.

For a good introduction to the code, I think the Aztec module is a solid starting point, it gives you an idea of how a lot of the library works and it’s stable.

Over the entire project, I think that the implementations of the minimal-encoders could use some rusting. In Java they were implemented in the Java way (everything is an object on the heap and happily lives forever if there is a reference and can refer to itself and its pals). I took the “easy” way out and implemented that entirely as Rc. That might be a bigger project though.

Nearly anything in common is also a good place to look (ECIStringBuilder being an exception, at least in part because I’m considering dramatically reworking how it behaves, bringing it more inline with the zxing-cpp approach, which I think I prefer over the one I have). The code isn’t exciting in common, but it’s pretty important. reedsolomon module included, it’s a core part of a lot of the library.

Speaking of ReedSolomon, I took the path of expediency when I ported it, so there are loads of inefficiencies and “not very rust” things I did in there. Doesn’t help that common was the first thing I ported so I hadn’t learned a lot of lessons yet.

I apologize for this comment being extremely long and rambling! I’ve been living with this code for so long it’s a process sometimes to read it objectively. Getting external eyes, like yours, on the code is going to be the single biggest contributor to its improvement!

@Asha20
Copy link
Contributor Author

Asha20 commented Feb 14, 2023

Thank you so much! There's no need to apologize. I'm grateful for the thorough answer and I didn't find it rambly at all. If anything, it further encourages me to dive in, now that I have a good idea of the places to check out.

Talk to you again soon!

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

No branches or pull requests

2 participants