Advertisement

Advice on coding style

Started by December 23, 2015 11:45 PM
45 comments, last by Hodgman 8 years, 11 months ago

where no "official" style is suggested

LOL

The ISOCPP guidelines have a whole "supporting section" (section NL) on stylistic crap. It's one of my main beefs with the document at present: I really feel that it's a low blow to smuggle that kind of nonsense into an ISOCPP guidelines document, even if they couch it with language indicating that it's only a suggestion. It lends far too much credence to things that are wholly optional, and will serve no purpose other than to erode practical freedoms over time. From an influential group, a wink can be as good as an edict.

IMNSHO there are only two rules for style:

  1. Regardless of the style you use, be consistent throughout the whole project.
  2. If someone is paying you, do what they tell you to do.

In any case, the guidelines themselves should be taken with a grain of salt - since it's still the early stages of a working document - but they do include a lot of good advice from a lot of highly experienced people.

void hurrrrrrrr() {__asm sub [ebp+4],5;}

There are ten kinds of people in this world: those who understand binary and those who don't.

The C++ core guidelines are not even close to what I would call an language's official style. The guide even says that this is only a suggestion if you have no better idea and that otherwise you should keep using whatever style you are comfortable with or whatever style your organization has chosen. I agree that the suggested coding style is questionable at best (since almost nobody uses it) but I think recommending a working coding style for beginners to start with is the right thing to do, otherwise they will just import whatever style they used in another programming language, which may or may not work with the C++ programming language.

A language's official coding style is something like a compiler or IDE enforced coding style that will get you looked down upon by the community if you don't respect. Use PascalStyle or iso_cpp to name a method in a Java program and see how Eclipse likes that. And in Ruby the capitalization of the first letter is very meaningful to the interpreter. That kind of thing doesn't exist in C++. (Which, I would add, is a good thing.)

Advertisement
In regards to comments the most important rule to remember is that a programmer is going to be reading your comment, you should assume that they are competent with the language and not point out things that would be learned by studying the language.

If anything comments should reflect what is actually happening, if you make a couple function calls in the middle of your code because you're verifying some data before you do something with it, write a comment saying that. At the very least you're going to be writing comments to yourself, even if you work solo on a project, because at some point you're going to look at some old code of yours and have no idea what it does, or only a faint idea. Also try to make your code self documenting, you don't need to point out that m_position is the position of an object but you might need to clarify that you're pulling some data from a database connection to do something with it that may not be obvious. Some things are easier to self-document than others.


at some point you're going to look at some old code of yours and have no idea what it does

Yusssss. And then shout, "Why didn't I comment this horrible mess!"

void hurrrrrrrr() {__asm sub [ebp+4],5;}

There are ten kinds of people in this world: those who understand binary and those who don't.

Thanks for all the replies. I've come up with a snag though and I have absolutely no idea why. I'm doing exercises on Operator Overloading (changing the operators function for classes) and have no idea why my time time::operator - (time t2) Isn't working correctly. All others work fine. It is not subtracting the first time from the second, but if I reverse it (fTemp = fTime2 - fTime1) it works fine... here's the snipped and give me your thoughts! (Even on coding style if I'm learning bad habbits). Cheers!


// Exercise 3
#include <iostream>
using namespace std;

class time
{
	private:
		int hours;
		int minutes;
		int seconds;
	public:
		time():hours(0), minutes(0), seconds(0)
		{}
		time (int h, int m, int s):hours(h), minutes(m), seconds(s)
		{}
		void timeDisplay() const;
		time operator + (time t2);
		void operator ++();
		void operator --();
		time operator - (time t2);
		time operator * (time t2);
};

time time::operator -(time t2)
{
	
	hours = hours - t2.hours;
	minutes = minutes - t2.minutes;
	seconds = seconds - t2.seconds;
	
	while(seconds < 0)
	{
		seconds += 60;
		--minutes;
	}
		
	while(minutes < 0)
	{
		minutes += 60;
		--hours;
	}
	
	
	return time(hours, minutes, seconds);
 } 
time time::operator *(time t2)
{
	hours *= t2.hours;
	minutes *= t2.minutes;
	seconds *= t2.seconds;
	
	while(seconds > 60)
	{
		seconds -= 60;
		minutes++;
	}
	while(minutes > 60)
	{
		minutes-= 60;
		hours++;
	}
	
	return time(hours, minutes, seconds);
} 

void time::operator --() 
{
	--hours;
	--minutes;
	--seconds;
	
	if(hours<0)
	{
		hours = 0;
		minutes += 60;
	}
	
	if(minutes > 60)
	{
		minutes -= 60;
		++hours;
	}
	if(seconds > 60)
	{
		seconds -= 60;
		++minutes;
	}
}

void time::operator ++() 
{
	++hours;
	++minutes;
	++seconds;
	
	if(seconds > 60)
	{
		seconds -= 60;
		++minutes;
	}
	if(minutes > 60)
	{
		minutes -+60;
		++hours;
	}
}

void time::timeDisplay() const
{
	cout << hours << ':' << minutes << ':' << seconds;
	cout << endl;
}
time time::operator +(time t2) 
{
	hours = hours + t2.hours;
	minutes = minutes + t2.minutes;
	seconds = seconds + t2.seconds;
	
	while (seconds > 60)
	{
		seconds -= 60;
		minutes++;
	}
	while (minutes > 60)
	{
		minutes -= 60;
		hours++;
	}
	return time(hours, minutes, seconds);
}

int main()
{
	time fTime1(12, 42, 56);	// Initialize some times
	time fTime2(1, 23, 42) ;
	time fTemp;
	
	fTemp = fTime1 + fTime2;
	
	fTemp.timeDisplay();
	
	++fTemp;
	fTemp.timeDisplay();
	
	--fTemp;
	fTemp.timeDisplay(); 
	
	fTemp = fTime1 - fTime2;
	fTemp.timeDisplay();
	
	fTemp = fTime1 * fTime2;
	fTemp.timeDisplay();
	
	return 0;
}
  1. It doesn't make sense to multiply a time by another time. It would make sense to multiply a duration by a scalar though.
  2. It doesn't make sense to increment or decrement this implementation of time because it's not unitary and it's not scalar.
  3. NO: operator + (time t2); YES: operator+(time t2);
  4. time t(0, 1000000, -12345); //invariants are not correctly enforced for m or s ranges
  5. while(seconds < 0) { seconds += 60; --minutes; } //use modulo and divide instead. also, you correct ranges in more than one place. make it a private member function
  6. time::timeDisplay(); //instead of writing functions that stream to cout, write functions that return string or take ostream& argument so that you can use other output methods.
  7. NO: operator+(time t2); YES: operator +(const time& t2); //const correctness is your friend
  8. Your compiler should be telling you about line 103. If not then increase your warning level.
  9. If the time/duration is negative how is that expressed? What if hours and minutes are positive and seconds is negative? What about hours being negative and the others positive? How does this work?
  10. If you implement a negation operator then you'll be able to implement the subtraction operator as "self + -other".
  11. You're using three values to express a single quantity. What if you were to use one large value (long long seconds?) and then just do the reporting differently?
void hurrrrrrrr() {__asm sub [ebp+4],5;}

There are ten kinds of people in this world: those who understand binary and those who don't.
Advertisement

I know making all these Overloaded Operators makes absolutely no sense, it's the questions in the book I'm reading to get you used to doing them properly. As for the protection, yes I understand that, but again, it's more about teaching me how to overload an operator.

It's only chapter 8 in Object-Oriented Programming in C++ by Robert Lafore (Great book so far). But my question still remains - if you run this through your compiler, the operator (-) doesn't work unless fTime2 is subtracted by fTime1. This is what is confusing me!

(I have a couple books to read after this one about programming techniques though!)

Here's a hint:

Returns expected result:


int main()
{
	time fTime1(12, 42, 56);
	time fTime2( 1, 23, 42);
	time fTemp;
	
	fTemp = fTime1 - fTime2;
	fTemp.timeDisplay();
}

Does not return expected result:


int main()
{
	time fTime1(12, 42, 56);
	time fTime2( 1, 23, 42);
	time fTemp;
	
	fTemp = fTime1 + fTime2;
	fTemp.timeDisplay();
	
	
	fTemp = fTime1 - fTime2;
	fTemp.timeDisplay();
}

Super-hint: Harking back to const correctness, operator-(time); should be a const function. The const correct prototype should be:


time operator-(const time& t2) const;

The compiler will flat out tell you the problem once that's the case.

void hurrrrrrrr() {__asm sub [ebp+4],5;}

There are ten kinds of people in this world: those who understand binary and those who don't.

My Dev C++ must be setup oddly because I put the const reference to my object of time and it didn't complain. But that being said, I see that my fTime1 variables are ALWAYS being changed.

So I have the option to A: Create local variables in the statements which will be destroyed afterwards, or create a temp object, then returning that object, which will also be destroyed afterwards. I can't believe I overlooked that!

Making the const reference to time t3 didn't make it complain, but once I made the whole declaration a const (time operator - (time t3) const) it yelled at me like crazy. Such a subtle mistake can make a mind go bananas. Something about learning though, and something I'll never repeat. Thank you so much for the help!

edit: I didn't even read the last const in your sentence. Well I figured it out regardless with awesome guidance. :)

That's exactly (part of) what const correctness does for you. It catches subtle errors like that which are otherwise valid code but don't do what you really want.

Good luck. Let us know if you have more questions. smile.png

void hurrrrrrrr() {__asm sub [ebp+4],5;}

There are ten kinds of people in this world: those who understand binary and those who don't.

This topic is closed to new replies.

Advertisement