hsf-training / hsf-training-cpp-webpage

C++ training
https://hsf-training.github.io/hsf-training-cpp-webpage
6 stars 11 forks source link

Introduction #7

Open giacomini opened 3 years ago

giacomini commented 3 years ago

This PR concerns the Introduction chapter. Apart from some typos, this PR:

chavid commented 3 years ago

I support strongly adding the link to Core Guidelines. And yes, let's remove this useless return 0 which makes any example longer. I am less convinced with '\n'. Yes, for those applications which are doing many outputs (is there so many such applications ?), this will make a big performance improvment. But in the general, I prefer the more explicit std::endl. Whatsoever, it sounds like a detail and I will agree the majority (and if so, try not to put std::endl everywhere...).

giacomini commented 3 years ago

std::endl always does a flush, see https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rio-endl and the notes at https://en.cppreference.com/w/cpp/io/manip/endl

giacomini commented 3 years ago

If we go for \n I need to add an explanation after the example

chavid commented 3 years ago

As said in the guidelines : "Apart from the (occasionally important) issue of performance, the choice between '\n' and endl is almost completely aesthetic."

I prefer the "aesthetic", which means readibility to me, of endl. I understand you prefer the aesthetic of \n, perhaps you feel the performance issue is not so occasionnal ?

@all, what do you think ?

glenn-horton-smith commented 3 years ago

Is the "\n" vs std::endl issue the only thing holding up merging this PR? I have a personal preference, but I prefer "either one is fine" even more. I don't think an explanation of "\n" would be needed after the example, any more than an explanation of std::endl would be needed: the existing text saying "Don’t worry about the pieces of the code here that you don’t understand yet" is sufficient to cover either one.

chavid commented 2 years ago

Yes, the "\n vs std::endl" is the only holding debate. And also the fact that lesson authors are all very busy and probably not actively checking the issues ;)

I would have prefered a global decision, so that the lesson examples are consistent. Yet it is not worth to block anything for such a low level detail. So yes, I buy a solution saying that we can use both.

P.S. : personally, it is something I am fighting in my everyday reviews : a C-like legacy optimization which is not worth in 99% of codes. I am afraid we wiil not avoid many of this experts controversies :)

giacomini commented 2 years ago

Hi. The reason I prefer \n is not asthetics, is awareness of what something means. I don't have additional arguments wrt to those already mentioned. If not everyone is convinced, the status quo wins, no big deal. I undo the second commit, I guess the PR should update accordingly, so that you can merge it (possibly squashing). Is that ok with you?

stale[bot] commented 1 year ago

This issue or pull request has been automatically marked as stale because it has not had recent activity. Please manually close it, if it is no longer relevant, or ask for help or support to help getting it unstuck. Let me bring this to the attention of @klieret @wdconinc @michmx for now.