search

osiewicz / Statula

Unix command line tool for statistics.
MIT License
7 stars 10 forks source link

Removed a couple of comments and reformatted code with clang-format. #1

Closed jaihindhreddy closed 7 years ago

jaihindhreddy commented 7 years ago

Please use clang-format on save. Manually formatting code is a waste of time.

osiewicz commented 7 years ago

Hey!
Sorry for not merging/responding for 12 days straight - I've been busy with my exam session.
Thank you for contributing to the project anyways! I will look through it this morning since I was about to go to sleep.
I don't know if I agree with the statement on formatting code by hand, but that is not relevant to the pull request.
Cheers!

Edit: Alright, I have went through your proposed changes - vast majority of those will make their way into the next commit (which will come before v0.2.0). One thing I cannot really agree with is bracing - I know I have just started holy war of styling guidelines, but this really made me realise that we need those guidelines in place. What irks me about proposed way of bracing is how your code handles loops and conditionals.

if (L < R)
    {

This is what I am talking about - as I have said, I agree with lion's share of your changes. Headers will stay as they are since some whitespace is always good (in fact, I will incorporate your function declaration style into incoming styleguide). What is so wrong about placing braces in separate line for loops and conditionals? Well. I think it blurs the difference between functions and said loops and conditionals (it doesn't really change anything from pure functionality perspective) - when I read the code function has higher priority than lesser tools and that is why I would rather keep this type of bracing reserved for functions.

As for formatting tools vs formatting by hand - I do not care as long as guidelines are met.
I am really thankful for what you did - and I can't really blame you for not meeting non-existant guidelines. In fact, people are bound to disagree on certain matters and I know that in future I will not always be able to enforce styling. However, since this project of mine is still small, I will make sure to prepare set of guidelines for contributors to follow.

jaihindhreddy commented 7 years ago

Fantastic. Definitely agree with you on the styling. That particular formatting style is not my thing either but I finally got tired of manually formatting code and started using clang-format with the webkit preset (AFAIR) and got used to it. I'm looking forward to the next version. I believe most of the streaming functionality you will find implementations of to get started. I found this gist with some useful stuff. https://gist.github.com/debasishg/8172796 Do take a look.