search

rxing-core / rxing

cRustacean Crossing
https://crates.io/crates/rxing
Apache License 2.0
195 stars 19 forks source link

Code rustification; where to start? #8

Open Asha20 opened 1 year ago

Asha20 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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!