C++

Forum

 
Template Class - coding style

Pages: 12
barliesque (17)

As I learn c++ I'm trying to decide on a coding style that's clean and portable.
I've just adapted a class I've been writing for practice into a template class. I'd be interested to hear any feedback about the coding conventions I've come up with here...

You'll notice I've kept the declarations and implementation split into two files, for the sake of making all my classes follow similar conventions. I wonder if this will be looked down upon, or if there are others who do this as well...?

Rectangle.h 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
 
#ifndef RECTANGLE_H
#define RECTANGLE_H

#include <string>
using namespace std;

template <class Number = float>
class Rectangle {

	public:
		Number x, y, width, height;
		
		Rectangle();
		Rectangle(Number x, Number y, Number width, Number height);

		virtual ~Rectangle();

		Number area();

		bool contains(const Rectangle& rect);
		bool contains(Number _x, Number _y, Number _width, Number _height);
		bool contains(Number pointX, Number pointY);
};

#include "Rectangle.cpp"

#endif // RECTANGLE_H



Finding the template syntax in implementation code to be visually crowded, I decided to use #define to keep the code as clean and readable as possible. What do you think?

Rectangle.cpp 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
 
#ifndef RECTANGLE_CPP
#define RECTANGLE_CPP

#include "Rectangle.h"

#include <iostream>
#include <cstdio>
#include <math.h>
#include <string>
#include <sstream>

using namespace std;

/**************************
**  TEMPORARY #DEFINES
***************************/
#define TMP__	template <class Number>
#define RECT	Rectangle<Number>

/*****************
**  CONSTRUCTORS
******************/
TMP__ RECT::Rectangle()
: x(0), y(0), width(0), height(0) {
}

TMP__ RECT::Rectangle(Number x, Number y, Number width, Number height)
: x(x), y(y), width(width), height(height) {
endl;
}


/****************
**  METHODS
*****************/

TMP__ Number RECT::area() {
	return width * height;
}

TMP__ bool RECT::contains(const RECT& rect) {
	return contains(rect.x, rect.y, rect.width, rect.height);
}
TMP__ bool RECT::contains(Number _x, Number _y) {
	return (_x >= x && _y >= y && _x <= (x + width) && _y <= (y + height));
}
TMP__ bool RECT::contains(Number _x, Number _y, Number _width, Number _height) {
	return (_x >= x && _y >= y && (_x + _width) <= (x + width) && (_y + _height) <= (y + height));
}

/********************
** #UNDEF TEMPORARIES
*********************/
#undef TMP__
#undef RECT

#endif

Duthomhas (13206)
IMO, what you are doing with that cpp file is very bad. Don't do that. Just stick everything in the header file, or if you must, use an extension like ".inc" or ".tcc" or the like for your #included file.

As for clarity, I don't think it helps much. It is just as easy to read it with the template stuff there. I prefer to format mine a little differently than yours, though.

Also, you have two off by one errors on line 45.
closed account (zb0S216C)
Personally, there's nothing new to me here. As for the source file, that's just plain hard to read. There is not enough comments to explain what's going on in you're code. I don't know why you put these in you're source code( this is new to me ):

1
2
3
4
 
#ifndef RECTANGLE_CPP
#define RECTANGLE_CPP

#endif


The string header has be included twice along with using namespace std;.

Overall, I wouldn't use the conventions you've used here.
barliesque (17)
Thank you for your reply. Much appreciated.

Okay. I won't split templates. I thought I might get that reaction. All I needed was to hear it from one person, and I'd drop it. Best stick to accepted convention for the sake of portability and other programmers being able to follow my code. I do like the idea of using a different filename extension.

On the subject of the #defines, for me they add *considerably* to the readability of the code. It's such an inefficient syntax that requires template <class Number> Rectangle<Number>:: before every single method. Now if the syntax were something like:
1
2
3
4
5
 
template <class Number> {
    Rectangle<Number>::myMethod() { ... }
    Rectangle<Number>::anotherMethod() { ... }
    Rectangle<Number>::someOtherMethod() { ... }
}

...that'd be nice, wouldn't it? But I'd best settle on learning the language rather than rearchitecting it.

Also, you have two off by one errors on line 45.

Well spotted. Thanks for that.
barliesque (17)
I was just looking at a template class that has to include this...
1
2
3
4
 
template<typename _CharT, typename _Traits, typename _Alloc,
template <typename, typename, typename> class _Base>
__versa_string<_CharT, _Traits, _Alloc, _Base>&
__versa_string<_CharT, _Traits, _Alloc, _Base>::

...before each and every definition, over and over. I find it difficult to locate the name of whatever's being defined with that huge mess in the way!

Here's a revision of my template class style example...

Rectangle.tcc 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
 
#ifndef RECTANGLE_H
#define RECTANGLE_H

#include <iostream>
#include <cstdio>
#include <math.h>
#include <string>
#include <sstream>
using namespace std;


template <class Number = float>
class Rectangle {

	public:
		Number x, y, width, height;
		
		Rectangle();
		Rectangle(Number x, Number y, Number width, Number height);
		
		Number area();
		
		bool contains(const Rectangle& rect);
		bool contains(Number _x, Number _y, Number _width, Number _height);
		bool contains(Number pointX, Number pointY);
		
		virtual ~Rectangle();
};

/**************************
**  TEMPORARY #DEFINES
***************************/
#define TMP__	template <class Number>
#define RECT	Rectangle<Number>


/*****************
**  CONSTRUCTORS
******************/
TMP__ RECT::Rectangle()
: x(0), y(0), width(0), height(0) { }

TMP__ RECT::Rectangle(Number x, Number y, Number width, Number height)
: x(x), y(y), width(width), height(height) { }


TMP__ Number RECT::area() {
	return width * height;
}


TMP__ bool RECT::contains(const RECT& rect) {
	return contains(rect.x, rect.y, rect.width, rect.height);
}
TMP__ bool RECT::contains(Number _x, Number _y) {
	return (_x >= x && _y >= y && _x < (x + width) && _y < (y + height));
}
TMP__ bool RECT::contains(Number _x, Number _y, Number _width, Number _height) {
	return (_x >= x && _y >= y && (_x + _width) <= (x + width) && (_y + _height) <= (y + height));
}


/********************
** UNDEF TEMPORARIES
*********************/
#undef TMP__
#undef RECT


#endif // RECTANGLE_H

Duthomhas (13206)
Templates weren't particularly designed to be split like usual classes, but it can be done... legally too.

The problem I had was with your file naming. If you use ".cpp" for stuff that is #included, you are asking for trouble with all kinds of tools and conventions.

As for the subject of #defines, for you they may add to the readability now, but for everyone else familiar with C++ code it actually detracts from the readability, and breaks convention. What you are doing is rearchitecting the language.

If you want to take a look at how things should work nicely, take a look at Bazzy's Bignum Challenge entry and how he codes it. http://home.comcast.net/~mlyn7576/bignum-challenge/#my-entry The same techniques will apply.

But, as you will.

BTW, NEVER use using directives in an #include file.

Also, your area() and contains() methods should be const.

Further, you assume that only rectangles with the same type are compatible. For example, I couldn't say:
1
2
3
4
5
 
  Rectangle <>    r0( 10, 10, 50, 50 );
  Rectangle <int> r1( 10, 40, 10, 10 );

  if (r0.contains( r1 ))
    ...
because they are templated over different types. You'd have to template the contains() methods as well:

1
2
3
4
5
6
 
template <class Number = float>
class Rectangle {
		...
		template <class Number2> bool contains(const Rectangle<Number2>& rect) const;
		template <class Number2> bool contains(Number2 x, Number2 y) const;
		template <class Number2> bool contains(Number2 _x, Number2 _y, Number2 _width, Number2 _height) const;
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
 
template <class Number>
template <class Number2>
bool
Rectangle <Number>
::contains(const Rectangle <Number2> & rect) const {
	return contains( rect.x, rect.y, rect.width, rect.height );
}

template <class Number>
template <class Number2>
bool
Rectangle <Number>
::contains(Number2 x, Number2 y) const {
	return contains( x, y, 1, 1 );
}

...

Sure, it is a lot to read, but it isn't difficult.

Hope this helps.
tition (887)
Templates weren't particularly designed to be split like usual classes, but it can be done... legally too.


@Duoas: I do that *all* the time - in fact this is the foundation of my programming style. In my math code, I often need to change the type of coefficients I work with (i.e. I need to change the base ring. My coefficients can be as simple as long integers, or as complicated as multivariate rational functions and multivariate quasipolynomials).

All my math classes (polynomials, monomials, matrices, etc.) are templated, with the template input representing the type of coefficient you work with (i.e. the ring).

@ barliesque:
I agree with everyone who thinks that your idea of using macros is very bad.

One example why it is bad: one year from now, I can 99% guarantee that you will not remember what your macro means. So, you will need to look up all the way to your .h file to find that out. So, the time you win now saving yourself from tedious copy+paste, you will lose later looking up what the heck did your macros mean. In addition, anyone working in team with you, using your .h file will be completely confused by your macro.
Last edited on
barliesque (17)

Thank you Duoas and Tition for your replies. Points well taken.

I decided to lay off trying to redesign template conventions and just focus on learning as much as I can first. I'm getting used to template syntax, and already even that large template declaration I posted above appears fairly readable to me now.

Further, you assume that only rectangles with the same type are compatible. For example, I couldn't say ... because they are templated over different types. You'd have to template the contains() methods as well.


Excellent! Yes, I had discovered the inability for a Rectangle<double> to convert to Rectangle<float> but didn't know how to solve that problem. You see, I didn't realize at first that different methods would require different template signatures, which is why it seemed at first like a reasonable thing to just use #define ...but now that I see each function has its own template needs, I realize what a terrible idea using #define's would be. Actually, I'm generally inclined to avoid #define's like the plague. :)

Another method in my class I'd omitted for brevity, I realized would have to be templated over two other types. ...Does this look right?
 
 
template <class Number2, Number3> Rectangle& interpolate(const Rectangle<Number2>& rectA, const Rectangle<Number3>& rectB, float i);


@Tition - I would be interested to see an example of your split-file template coding style. Perhaps you could post a small extract? ...and what filename extensions do you use? As a single file, I was thinking of using *.t ...being similar to *.h for headers. Would that be generally understood by others, do you think?


Also, your area() and contains() methods should be const.
 
 
template <class Number2> bool contains(const Rectangle<Number2>& rect) const;



I hadn't yet learned this usage of const until you pointed it out. I'm getting more out of this thread than I'd bargained for!

Again, thanks for your feedback.
Ozge (2)
#include "stdafx.h"
#include <iostream>

using namespace std;

template < class Data > class MyNode
{
Data *pdata;
MyNode*pleft;
MyNode*pright;
MyNode<Data>(Data*p):pData(p){}
public:
bool ISLE(Data*p)
{
return pData->IsLE(p);
}
};

template < class Data > class MyTree
{
MyNode<Data> *pRoot;

void Insert(MyNode<Data>*&pRoot,Data* pNew){
if(!pRoot){
pRoot=new MyNode<Data>(pNew);
return;
}
if(pNew->IsLE(pNew))
Insert(pRoot->pRight,pNew);
else
Insert(pRoot->pLeft,pNew);
}
public:
void Insert(Data*pNew){
Insert(pRoot,pNew);
}
};

int _tmain(int argc, _TCHAR* argv[])
{


return 0;
}





What does this mean?My professor did this in the class but i couldn't understand the purpose of writing this templates so that i could'nt fill the main block.What should i write within the main code to understand what this means???
Duthomhas (13206)
A template allows the compiler to create code for you.

I could write a "point" class holding two integers:

1
2
3
4
5
 
struct point_i
  {
  int x, y;
  point_i( int x = 0, int y = 0 ): x( x ), y( y ) { }
  };

Later, I could want it to work over floats:

1
2
3
4
5
 
struct point_f
  {
  float x, y;
  point_f( float x = 0, int y = 0 ): x( x ), y( y ) { }
  };

Each time, I have to write a new class.

1
2
 
  point_i p1( 10, 12 );
  point_f p2( 1.2, 2.7 );


With templates, I can have the compiler do all the work for me, and the template is part of the type too, so I can use the same name:
1
2
3
4
5
6
 
template <typename T>
struct point
  {
  T x, y;
  point( const T& x, const T& y ): x( x ), y( y ) { }
  };
1
2
 
point <int> p1( 10, 12 );
point <float> p2( 1.2, 2.7 );

Hope this helps.
barliesque (17)
Ozge's change of subject kind of hijacked the thread away from my last post. I had a couple of questions:

@Duoas...
Does this declaration look right?
 
 
template <class Number2, Number3> Rectangle& interpolate(const Rectangle<Number2>& rectA, const Rectangle<Number3>& rectB, float i);


@Tition - I would be interested to see an example of your split-file template coding style. I'm Perhaps you could post a small extract?

...and to reiterate, I plan to generally avoid #define's like the plague. :)

Duthomhas (13206)
Typically, no response means, "yes."

So, yes, it looks more or less correct. Watch the warnings your compiler spits out at you to fix any syntax errors you have. Also, don't forget your const.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
 
template <class Number>
class Rectangle  {
	template <class Number2, class Number3>
	Rectangle&
	interpolate(const Rectangle<Number2>& rectA, const Rectangle<Number3>& rectB, float i)
	const;
};

template <class Number>
template <class Number2, class Number3>
Rectangle<Number>&
interpolate(const Rectangle<Number2>& rectA, const Rectangle<Number3>& rectB, float i)
const {
	...
}

Keep in mind that this has its problems, too. You will find that if your rectangle uses a more limited numeric type than the argument rectangles, then your results will be off.

What exactly are you trying to do here? Perhaps there is a better way...
barliesque (17)
Typically, no response means, "yes."

I'm still a beginner here, so don't hold back with reassurances if I get it right. ;)

The idea of this function is to set all of the values of this Rectangle to be an interpolation between rectA and rectB. So, if I understand that usage of const, it shouldn't be there, since the properties of the rectangle will be changed.

As for mismatched type Rectangles, I had given that some thought, but I'm not sure there's any real problem ...if I cast to all values to float for the calculations, and then cast back to whatever this is.
Duthomhas (13206)
If you do that, though, why template the type of the class. Why would the user want a Rectangle <double> if you are just going to cast to float? Going the other way, from float to int, also has roundoff issues.

I still don't know what you are really doing with this "interpolation." Also, copying a rectangle is cheap enough that there really is no reason to do an interpolate-assign trick -- it will confuse your users.

1
2
3
4
5
 
Rectangle <> r0;
Rectangle <> r1( ... );
Rectangle <> r2( ... );

r0.interpolate( r1, r2 );
The reader will be confused by that last line. Why is that r0 there if we are calculating the interpolation of r1 and r2? Are we calculating the interpolation of three rectangles? Where is the result going? It is better encapsulated to say:

  5 r0 = r1.interpolate( r2 );

or something along those lines.
barliesque (17)
The idea is that this function would assist some sort of transition from one rectangle to another. So, something like this would be a typical usage...

1
2
3
4
5
6
7
8
 
Rectangle<> start(...);
Rectangle<> finish(...);
Rectangle<> transition;

for (float i(0.0f); i <= 1.0f; i += 0.01f) {
    transition.interpolate(start, finish, i);
    drawSomething(transition);
}


If I cast all values to double and then back to the type of this it should function appropriately. If start and finish are <float> but transition is <int>, then the user should be expecting rounding off to occur. Certainly it's important to understand how this function is meant to be used.
Duthomhas (13206)
Ah, I understand. You want a linear interpolator.

The interpolator itself is better managed by another class, otherwise you will have jumpy results. You can then reposition the rectangle (via a moveto() or translate() or some like method). The rectangle itself will still keep its lower precision, but the interpolator will keep state in the higher precision. Notice also that this will require only one linear manipulator instead of four, and will keep the transformation state separate from your shape state.

For an example of an interpolator (with some error, admittedly), see this one I wrote here:
http://www.pygame.org/wiki/Interpolator?parent=CookBook

It is in Python, but it is easily translatable to C++. The documentation/commentary is very complete and will help you along. Keep in mind that the documentation refers to "vectors" -- which are N-dimensional points, and not the C++ std::vector.

It has some advanced shape modification controls, and it comes with a little test program to let you see it in action. (You need Python and PyGame to see it work, though.)

There are other interpolators in the Cookbook there, but they have some serious bugs, JSYK.

Good luck!
barliesque (17)


Actually, I've written a tweening engine before, just not in C++. This function isn't intended to have built-in easing, which would be more appropriately placed in a tween library of easing functions. This interpolate() function would be useful combined with a class like that.

You know I don't think I've ever looked at Python code before. It's kinda ugly to my ecmascript eyes! No offense. :)


Anyway...

I'm having a problem writing a copy constructor for this template class of mine. For some reason this copy constructor never gets called...

from Rectangle.h 
1
2
3
4
5
6
 
template <class Number>
template <class Number2>
Rectangle<Number>::Rectangle(const Rectangle<Number2>& rect)
: x(rect.x), y(rect.y), width(rect.width), height(rect.height), _id(counter++) {
	std::cout << "Copy a Rectangle " << rect.id << " to new Rectangle " << _id << std::endl;
}


from main.cpp 
1
2
 
Rectangle<> rect1 = Rectangle<>();
Rectangle<> rect2(rect1);


A copy of the rectangle is made, but not with the copy constructor I've written... huh???
Last edited on
cran (218)
Duoas, I have a question... you posted 

1
2
3
4
5
 
struct point_f
  {
  float x, y;
  point_f( float x = 0, int y = 0 ): x( x ), y( y ) { }
  };


and underneath with the template you have const T& x or something like that, I've been trying to use the convention:


1
2
3
4
5
 
struct point_f
  {
  float x, y;
  point_f( const float& x = 0, const int& y = 0 ): x( x ), y( y ) { }
  };


.. well, except I usually write it const float &x.. either way, I use that unless I need to either change the var as a reference or I need to use the data in the var and not reference it... is it better to not have const & for initialized vars?
cran (218)
I was about to complain about you saying "never use using directives", but realized that you said in the include file. Yeah, I agree with that. I like to avoid them altogether, but probably should use things like using std::cout or something since I have std::cout all over the place.

Oh, I wanted to ask another question...

1
2
3
4
 
#ifndef RECTANGLE_CPP
#define RECTANGLE_CPP

#endif


I use that all the time instead of #pragma once since I thought it wasn't really standardized... Should I be using pragma instead?
Last edited on
acorn (276)
#pragma once is a non standard thing

here is a quick article about it. my main point is its not part of the standard

http://blogs.msdn.com/b/ericflee/archive/2008/04/17/pragma-once.aspx

*edit to be more clear
Last edited on
Pages: 12