jcelaya / hdrmerge

HDR exposure merging
http://jcelaya.github.io/hdrmerge/
Other
355 stars 78 forks source link

request: increase code readability #180

Open fanckush opened 5 years ago

fanckush commented 5 years ago

I'm really interested in this project and would love to contribute and start closing some of the issues. To be honest, the code isn't very understandable, many variables don't have proper naming and I think that makes it difficult for new people who didn't initially write the code. Take this function for example: https://github.com/jcelaya/hdrmerge/blob/3ad6d36c517ddb7f164799c98800bdccb2e5b9f2/src/Image.cpp#L100

uint16_t * usePixels = &data[-reldy*width - reldx];
const uint16_t * rUsePixels = &r.data[-relrdy*width - relrdx];
// some code...
double nv = rUsePixels[pos];
double v = usePixels[pos];

Maybe that's how C++ devs write their code, I'm not sure as I've never worked on a C++ project. If what I'm saying makes sense and you agree with me, maybe we can do it bit by bit a function/method at a time by renaming variables and adding some more comments, specially for the mathematical components.

Benitoite commented 5 years ago

In college I learned the type_nameInCamelCase convention in ANSI C class. @heckflosse @Floessie etc are probably the C/C++ experts if it comes to drafting a CONVENTIONS.md.

fanckush commented 5 years ago

Cool. from my point of view, any convention is fine really as long as it's verbose :)

heckflosse commented 5 years ago

Imho @Floessie is the expert for coding conventions.

Floessie commented 5 years ago

I think we all agree that having strong conventions might hamper creativity and are hard to enforce on an existing project with many contributors. Still there should be some guidelines...

For RawTherapee, we have a cautiously moderated CONTRIBUTING.md with some rough guidelines not meant to be too interfering.

If what I'm saying makes sense and you agree with me, maybe we can do it bit by bit a function/method at a time by renaming variables and adding some more comments, specially for the mathematical components.

This sounds reasonable. IMHO comments should give an overview of the algorithm. If the identifiers are clearly describing the intent, single lines don't need to be commented.

When refactoring don't only rename identifiers but also think about improvements:

Although I'm not an official contributor to HDRMerge I'm very interested in it from a user's perspective and am really looking forward to your enhancements. :+1:

HTH, Flössie

fanckush commented 5 years ago

I made this PR #181