code review

[TheHardew]

I've just discovered codeforces.com and stared writing solutions to the problem sets, as I am not too good at writing something other than c++ books examples.

Could you please review my solution for problem 1B? It was written for vs 2015, hence #define _CRT_SECURE_NO_WARNINGS , but it runs on gnu as well, just delete the define directive. Please tell me what I did wrong (probably a lot) and what well (if anything). Thanks in advance.
The code: http://pastebin.com/SXMGRzyk

[TheIdeasMan]

Hi,

Missing a link to your code :+) A link to the problem would save a small amount of time, and be convenient

Regards :+)

[TheHardew]

What a fool I am! Sorry, the post is updated with the link. I don't have any problems, it ran well through site's checking systems, I just want your opinion about the code. Thanks.

[TheIdeasMan]

Hi,

The problem description, here:

http://codeforces.com/problemset/problem/1/B

In my very humble opinion, your code looks fine :+) there are many, many far more experienced people than me


With the cstdio vs iostream thing, I gather that the STL is supposed to be zero overhead. In other words, any small overhead present in STL is probably not worth worrying about.

One probably pedantic thing is with any of the scanf functions, make use of the return value to see that it worked.

Good Luck!!

[jlb]

// cstdio is faster, right? I don't know if I should mix it with iostream

IMO you should stick with the C++ streams, you're not going to see much speed increase by switching between the cstdio functions and the C++ streams so stick to the C++ streams. The C++ streams are much less error prone since you don't have to worry about correctly matching the format specifier to the variable. If you stick with C++ streams you can un-sync the streams from stdio to get a small improvement in speed.

http://www.cplusplus.com/reference/ios/ios_base/sync_with_stdio/

[TheHardew]

That's bad, I love std::printf, it has really nice and simple formatting options and you don't have to type so much, for example printf("%04u", num); vs cout << setfill('0') << setw(4) << num, plus the cout version is simplified with iomanip and using namespace std;. But if you say it's bad, I'm gonna stop using that. What do you think about declaring pointers and references? Where to put the '*', '&'?T *var or T * var?

[jlb]

and you don't have to type so much,

You shouldn't really be worried about "typing so much", strive to make your program as humanly readable as possible. And if you expect other C++ programmers to easily read and understand your programs you should stick with C++ features since many C++ programmers may not be very familiar with the <cstdio> functions. But try not to mix the <cstido> functions and the C++ streams, this is really frowned upon. If you're going to use <cstdio> use it everywhere.

What do you think about declaring pointers and references? Where to put the '*', '&'?T *var or T * var?

It really doesn't matter to me, however associating the pointer or reference with the type is probably the more acceptable notation int* name, int& name. Also many people will accept int *name, int &name as well, but reject int * name outright. But this is all subjective, as long as you are consistent most people can live with any of the methods.

[TheHardew]

I don't use int* name; cuz it can be a problem with a comma: int* name, name2; - one pointer to int and one int. I used to write int *name;, but then VS auto-function generation generates int * name, so I thought it might be better for whatever reason.

[jlb]

I don't use int* name; cuz it can be a problem with a comma: int* name, name2; - one pointer to int and one int.

That's why it's considered better to have only one declaration per line.

[TheHardew]

I didn't know it's one of those allowed-but-bad things. Is this

int counter, 
    childCounter, 
    percentComplete;

also bad? I like to use the comma, because it lets you change type of a few similar variables at once.

[jlb]

That's the same as:
int counter, childCounter, percentComplete;

There is nothing actually wrong with the above, as long as there are all the same exact type (no pointers, etc.). However in C++ it is considered a better practice to declare the variables near the time of first use (which you appear to be practicing), instead of in one lump at the beginning of scope.

I like to use the comma, because it lets you change type of a few similar variables at once.

Again aversion to typing shouldn't be a real consideration.

[JLBorges]

> I like to use the comma, because it lets you change type of a few similar variables at once.

Not really; the type would still have to be changed at many places.

float average( std::vector<float> seq ) ;

// more functions using vector of float

int main()
{
    // change type from float to double?
    std::vector<float> first_vec, second_vec, third_vec ;
}

If a change of type is a possibility, create a type alias, and then use the type alias everywhere else.

using data_base_key_type = unsigned int ;
typedef unsigned int data_base_key_type ; // same as above 

EDIT: This may be of interest
Is "int* p;'' right or is "int *p;'' right? http://www.stroustrup.com/bs_faq2.html#whitespace

[TheHardew]

I know it's the same, just quick search at stackoverflow showed a thread where someone was advising that. I'm not gonna use it any more if it's bad.

it is considered a better practice to declare the variables near the time of first use

I know it's preferred, that's why I tried to do it this way in the project, though I'm used to declaring variables at the top of the function. Another on thing to adjust.

EDIT:
ninja'd by JLBorges. Thank you, now I know why STL declares aliases in every class. I still have to read the link you posted.

EDIT2:
I read about that emphasising when I was starting to learn c++ from c++ primer plus 6th edition, but than I saw declaring two variables in one line and started declaring pointers the c way. Now I'm gonna go back to int* p and stop using declaring two variables using one type keyword. Thanks.