search

jcelaya / hdrmerge

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

Image stack refactors #188

Open fanckush opened 4 years ago

fanckush commented 4 years ago

I would've liked to have the whole class refactored and then make a PR but I'm thinking if I make frequent PRs it will reduce the possibility of unnecessary merge conflicts.

I come from Python / JS , what I learned from Python is that things should be as clear as possible :). This is my first attempt at refectory C++ so please throw all your constructive criticism at me now while I haven't formed my preferences and biased opinions on C++ conventions yet.

PR reason: #180

fanckush commented 4 years ago

can anyone give this a look :)

Beep6581 commented 4 years ago

Ping @Floessie @heckflosse

ff2000 commented 4 years ago

I'm not bias against CamelCase or under_score notation. But IMO it is better to stick with what is already used, and as far as I can see that's CamelCase here in HdrMerge. Of course it's up to the owner/main devs to decide, but I have mixedFeelings with mixed_notation ;) (As @Floessie already reviewed this PR and he didn't mention this it likely is OK.)

Floessie commented 4 years ago

@ff2000 I didn't want to belabor mixed notation. Of course it would be nice to have a common style, but this shouldn't be strictly enforced, as this probably impedes contributors. It's up to @fanckush.

fanckush commented 4 years ago

Good point, I personally still like this mixed notation because it adds an extra level of semantic separation between methods and variables which may reduce the ambiguity even a tiny bit.

This is just the beginning so it would be nice to decide now before i continue refactoring the rest. If everyone prefers camelCase that's fine by me too, maybe my python background is pulling me towards under_scores for variables but i'm used to both.

fanckush commented 4 years ago

Please don't merge this PR just yet, some time has passed and i totally agree with what you are saying. will strictly use camelCase and drop snake_case because it's more consistent and introduces less changes to the code base 👍 Also in the chance that i don't refactor all the code, everything will be camelCase :)