Paranoid about style/elegance and function size, please quell

[wired]

Hi,

I've just taught myself C++, so I haven't learnt much about style or
the like from any single source, and I'm quite styleless as a result.
But at the same time, I really want nice code and I go to great
lengths to restructure my code just to look concise and make it more
manageable.
When I say this, I'm also referring to the way I write my functions.
It seems to me sometimes that I shouldn't have many void functions
accepting addresses and changing their values for example, but rather
a double function that returns the calculated value. i.e.
void Change(Type& variable);
should be replaced with
Type Change(Type variable);

I just don't know what's better code (in terms of speed, style, etc.).

I'm moving into a more mature phase of my program, and before I
continue I would like jsut a final verdict on my code. I'm looking for
comments about whether my functions should be more simple (i.e. too
much happens in each), whether I should rather return value instead of
alter through references passed, whether my general style is
inelegant. I'm also concerned about my class structures. But I'll show
you _some_ code and let you decide.

Please note that I'm not asking you to go through the actual logic of
all this code, but rather just calls to functions, classes and the
like.

Thanks.

Davin.

//---------------------
//Globals.cpp - just some global variables needed by most functions
#include <vector>
#include "GLFiles.h"
#include "Derived.h"

using std::vector;

//quadratic to draw cylinders, spheres etc.
GLUquadricObj *quadratic;

//window dimensions
int WIDTH = 1024;
int HEIGHT = 768;

//table dimensions
float length = 25.15f; // 251.5cm
float width = 13.84f;
float height = 8.13f;
float goal = 4.0f;

float rest = 1.5f; // width between esdge and tabletop

// Misc

int step = 150; // number of steps the collision detection will
deconstruct each translation into in checking for a collision

Mallet* Mallet1;
Mallet* Mallet2;
vector <Puck*> Pucks;

double friction = 0.985;
int mouse_sens = 4; // 4-10, 10-least sensitive

//---------------------
//Abstract.h - two abstract classes, some derived classes will be
inherited from both
#ifndef Abstract_Classes
#define Abstract_Classes

#include "Globals.h"

using namespace std;

class Movable;
class Drawable;

void AddDraw(Drawable* draw);
void AddMove(Movable* move);

extern double friction;

class Drawable
{

public:

Drawable() {
AddDraw(this);
}

virtual void Draw() = 0;

};

class Movable
{

public:

Movable(double x1, double y1, double r, Controller cont) {

AddMove(this);

control = cont;

x = x1;
y = y1;

x_change = 0.0;
y_change = 0.0;

radius = r;

}

double next_x() { return x + friction * x_change; }
double next_y() { return y + friction * y_change; }

double change_x() { return x_change; }
double change_y() { return y_change; }

double now_x() { return x; }
double now_y() { return y; }

double get_radius() { return radius; }
double get_mass() { return mass; }

Controller controller() { return control; }

friend void Collide();
friend void HitTable(Movable* obj, double change);

void Replace(); // reset its position (after a goal for example) and
velocity

virtual void Move() = 0;

protected:

Controller control;

double radius;
double mass; //used as a comparison between inherited types (as well
as the physics)

double x;
double y;

double x_change;
double y_change;

};

#endif

//------------------
//Derived.h - two derived classes
#ifndef Derived_Classes
#define Derived_Classes

#include "Abstract.h"

class Mallet: public Drawable, public Movable
{

public:

Mallet(double x, double y, Controller cont): Movable(x,y, 0.76, cont)
{
mass = 3; // 3kg
}

void Draw();
void Move();

};

class Puck: public Drawable, public Movable
{

public:

Puck(double x, double y): Movable(x,y, 0.7, physics) {
mass = 0.03; // 30g
}

void Draw();
void Move();

};

#endif

//----------------
//Derived.cpp - the functions for Derived.h
#include <vector>
#include <cmath>

#include "GLFiles.h"
#include "Derived.h"

#include "GeoMotion.h"

using namespace std;

//MalletXXX works when a mallet isnt hitting a XXX
void MalletWall(GeoMotion& coords); // if the object has hit a wall,
it returns the appropriate variable, else it returns the same variable
value
void MalletSquash(GeoMotion& coords, Mallet* mal); // has it hit a
squashes puck against the wall (for example)?
void MalletBound(GeoMotion& coords, Mallet* mal); // if its in your
half - y includes the position + change + radius

GeoMotion MouseCont(Mallet* mal); // mouse control
GeoMotion AICont(Mallet* mal); // AI control

bool Over(double coord, double fixed);

extern GLUquadricObj *quadratic;

extern vector<Drawable *> DrawList;
extern vector<Movable *> MoveList;

extern float length;
extern float width;
extern float rest;

extern int step; // to disect the mallets movement if its going into
another object that is unmovable

extern vector<Puck*> Pucks;

extern double friction;
extern int mouse_sens;

extern int WIDTH;
extern int HEIGHT;

void Puck::Draw() {

glPushMatrix();

glColor3f(0.0f, 0.0f, 0.0f);

glTranslatef(x, 0.0f, y);
glRotatef(90, 1.0f, 0.0f, 0.0f);

gluCylinder(quadratic, radius, radius,0.1f,32,32); // main outside
gluDisk(quadratic,0.0f, radius,32,32); // disc covering top
gluCylinder(quadratic, 0.2, 0.2,0.11f,32,32); // nice looking ring in
the middle

glPopMatrix();

return;

}

void Puck::Move() {

x_change *= friction;
y_change *= friction;

x += x_change;
y += y_change;

return;

}

void Mallet::Draw() {

double OuterHeight = 0.25; // outer mallet height
double WallDown = 0.1; // how far down the base of the handle is from
the outer height
double WallIn = 0.1; // how far in the outer wall comes
double HHeight = 0.4; // handle heigth
double HWidth = 0.6;

glPushMatrix();

glColor3f(0.0f, 0.0f, 0.0f);

glTranslatef(x, 0.0f, y);
glRotatef(-90, 1.0f, 0.0f, 0.0f);

gluCylinder(quadratic, radius, radius, OuterHeight ,32,32); //
outside wall

glTranslatef(0.0f, 0.0f, OuterHeight);
gluDisk(quadratic, radius-WallIn, radius,32,32); // top of the wall

glTranslatef(0.0f, 0.0f, -WallDown); // has been rotated, so z axis
is where y used to be, and -
gluCylinder(quadratic, HWidth/2 , radius-WallIn, WallDown,32,32); //
connection from handle to top of wall
gluCylinder(quadratic, HWidth/2 , HWidth/2, HHeight ,32,32); //
handle

glTranslatef(0.0f, 0.0f, HHeight);
gluSphere(quadratic, HWidth/2 ,32,32); // handle nob

glPopMatrix();

return;

}

void Mallet::Move() {

GeoMotion coords(this);

if (control == mouse)
coords = MouseCont(this);
else if (control == AI)
coords = AICont(this);

x = coords.x;
y = coords.y;

x_change = coords.x_change;
y_change = coords.y_change;

return;

}

GeoMotion MouseCont(Mallet* mal) {

GeoMotion coords(mal);

POINT mouse;
GetCursorPos(&mouse); // from windows.h

double tmpX = mouse.x - WIDTH/2; // centre screen gives large
coordinates, but coordinates in the centre should be 0,0 - without
this, centre coords will be like 512,384
double tmpY = mouse.y - HEIGHT/2;

coords.x_change = (tmpX / WIDTH * width)/mouse_sens; // get it as a
fraction of the screen, then make that fraction relative to the table
dimension, then divide by the sensitivity
coords.y_change = (tmpY / HEIGHT * length)/mouse_sens;

//continually send the coords to a function and have values updated
MalletWall(coords); // if its hitting a wall, return the change that
it should have to go max without hittin, else return current change
MalletBound(coords, mal);
MalletSquash(coords, mal);

coords.x = coords.x + coords.x_change; // make the x position its
curretn position plus its newly calculated change
coords.y = coords.y + coords.y_change;

SetCursorPos(WIDTH/2, HEIGHT/2);

return coords;

}

GeoMotion AICont(Mallet* mal) {

GeoMotion coords(mal);

coords.x_change = Pucks[0]->now_x() - coords.x;
coords.y_change=0;
MalletBound(coords,mal);
coords.x += coords.x_change;
coords.y += coords.y_change;

return coords;

}

void MalletBound(GeoMotion& coords, Mallet* mal) { // bound to own
half

double littlebit = 0.3;
double bound = mal->controller() == mouse? 0 + Pucks[0]->get_radius()
+littlebit : 0 - Pucks[0]->get_radius() -littlebit; // boundary,
furthest forward it can go. 0 is halfway

if (mal->controller() == mouse &&
coords.y+coords.y_change-coords.radius <= bound)
coords.y_change = bound - (coords.y-coords.radius);
else if ( mal->controller() != mouse && mal->controller() != physics
&& coords.y+coords.y_change+coords.radius >= bound)
coords.y_change = bound - (coords.y+coords.radius); // make distance
to be just touching

return;

}

void MalletSquash(GeoMotion& coords, Mallet* mal) {

for (int i=0; i<MoveList.size(); ++i) // if its being squashed
against a wall..
{
if (MoveList[i] != mal && MoveList[i]->controller() == physics) //
if its not the same address (in which case of course they will collide
and this will return false) and if its changeable on collision, like a
puck
if (sqrt( pow(coords.x+coords.x_change-MoveList[i]->next_x(), 2.0)
+ pow(coords.y+coords.y_change-MoveList[i]->next_y(), 2.0) ) <
coords.radius+MoveList[i]->get_radius()) // if (using distance
formula) its not hittign the object, if it is, their distance b/w
radii will be less than sum of radii
if ( Over(coords.x+coords.x_change, width/2-rest
-2*MoveList[i]->get_radius()-coords.radius) == true ||
Over(coords.y+coords.y_change, length/2-rest
-2*MoveList[i]->get_radius()-coords.radius) == true)
for (int j=0; j<=step; ++j)
if (sqrt( pow(coords.x+ j*coords.x_change/step
-MoveList[i]->next_x(), 2.0) + pow(coords.y+ j*coords.y_change/step
-MoveList[i]->next_y(), 2.0) ) <
coords.radius+MoveList[i]->get_radius())
{
coords.x_change = (j-1)*coords.x_change/step; // take the one
right before collision, otherwise the object will bounce back INSIDE
the mallet
coords.y_change = (j-1)*coords.y_change/step;
}
}

return;

}

void MalletWall(GeoMotion& coords) {

if (coords.x+coords.x_change + coords.radius > width/2-rest) // if
its going over the side wall
coords.x_change = width/2-rest -coords.radius - coords.x;

else if (coords.x+coords.x_change - coords.radius < -width/2+rest)
coords.x_change = -width/2+rest + coords.radius - coords.x; // make
it go right up to the wall

if (coords.y+coords.y_change + coords.radius > length/2-rest) // if
its going over the front/back wall
coords.y_change = length/2-rest -coords.radius - coords.y;

else if (coords.y+coords.y_change - coords.radius < -length/2+rest)
coords.y_change = -length/2+rest + coords.radius - coords.y; // make
it go right up to the wall

return;

}

void Movable::Replace() {

x_change = 0;
y_change = 0;

x = 0;

if (y<0) // if it needs to be reset on the other side
y = -length/4;
else
y = length/4;

double inity = y;

bool overlap;
do {
overlap = false;

for (int i=0; i< MoveList.size(); ++i)
if (MoveList[i] != this && sqrt( pow(y - MoveList[i]->y, 2.0) +
pow(x - MoveList[i]->x, 2.0) ) < radius + MoveList[i]->get_radius())
{
overlap = true;

x >= 0? x=-x-radius/2 : x=-x; // if its on the right, switch it to
the left, if its on the left sidemove it one object diameter across
and retry

if (x < -width/2+rest + radius) // dont keep replacing it, so
check its not off the edge, but make sure you dont place it too close,
at least a radius away
{
x = 0;

y = y >= inity? y - 2*(y-inity) -radius/2 : y + 2*(inity-y); //
switch sides each time, done by subtracting from y twice its distance
from length/4 (starting point) thereby reflecting it by this point

if (y < radius || y > -radius)
{
y=inity;
overlap = false; // we have placed it everywhere in the whole
half and nothing works, pretend there nothing wrong and set it in a
goal position to redo this procedure next iteration
}
}

break;
}
} while (overlap == true);

return;

}

void AddDraw(Drawable* draw) {
DrawList.push_back(draw);
return;
}

void AddMove(Movable* move) {
MoveList.push_back(move);
return;
}

//----------------
//DrawMotion.cpp - handles functions like drawing each frame,
collision detection, and how objects should reflect (both speed and
direction)
#include <vector>
#include <cmath>

#include "Derived.h"
#include "GLFiles.h"

using namespace std;

void SetupObj();
bool LoadPucks(int pucks);
bool LoadMallets();
void ReplacePucks();
void CloseGL();
void DrawObj();
bool Over(double coord, double fixed);
int CircleCol(Movable* obj1, Movable* obj2);
int FixedCol(Movable* obj, double x, double y);
double GoalCoord(double coord, double fixed);
void HitTable(Movable* obj, double change);
double NewVel(double vel1, double vel2, double m1, double m2);
void Collide();
void Reset();
void NextFrame();

extern Mallet* Mallet1;
extern Mallet* Mallet2;
extern vector<Puck*> Pucks;

extern float length;
extern float width;
extern float height;
extern float rest;
extern float goal;

extern int step;

vector<Drawable *> DrawList;
vector<Movable *> MoveList;

void SetupObj() {

ShowCursor(false);

if (LoadPucks(1) != true || LoadMallets() != true) // LoadPucks first
because mallet AI is used to check where the puck is and it will move
the puck first this way, and the AI can check
exit(-1);

ReplacePucks();

return;

}

bool LoadPucks(int pucks) {

Puck* tmpPuck;

for (int i=0; i<pucks; ++i)
{
try {
tmpPuck = new Puck(0,5);
Pucks.push_back(tmpPuck);
}
catch(std::bad_alloc nomem)
{
return false;
}
}

return true;

}

bool LoadMallets() {

try {
Mallet* Mallet1 = new Mallet(0,6, mouse);
Mallet* Mallet2 = new Mallet(0,1, AI);
}
catch(std::bad_alloc nomem)
{
return false;
}

return true;

}

void ReplacePucks() {

for (int i=0; i<Pucks.size(); ++i)
Pucks[0]->Replace();

return;

}

void CloseGL() {

ShowCursor(true);

delete Mallet1;
delete Mallet2;

for (int i=0; i<Pucks.size(); ++i)
delete Pucks[i];

return;

}

void DrawObj() {

for (int i=0; i < DrawList.size(); ++i)
DrawList[i]->Draw();

return;

}

bool Over(double coord, double fixed) { // has coord gone beyond fixed
in both positive and negative dimensions

if (coord > fixed || coord < -fixed)
return true;

return false;

}

int CircleCol(Movable* obj1, Movable* obj2) { // returns 0 if there no
collision, else it returns the step of the process where the collision
was

double xdist1 = obj1->now_x(); // its distance in the x direction
after any given number of steps through the process of deconstruction
double ydist1 = obj1->now_y();
double xdist2 = obj2->now_x();
double ydist2 = obj2->now_y();

double tmpX1 = obj1->change_x();
double tmpY1 = obj1->change_y();
double tmpX2 = obj2->change_x();
double tmpY2 = obj2->change_y();

for (int i=1; i<=step; ++i)
{
if (Over(xdist1 +tmpX1/step, width/2-rest -obj1->get_radius()) ==
true) // if any of the NEXT distances (thats why one step has been
added) are beyond a boundary, change the direction
tmpX1 *= -1;
if (Over(ydist1 +tmpY1/step, length/2-rest -obj1->get_radius()) ==
true && (xdist1+tmpX1/step >= goal/2 || xdist1+tmpX1/step <= -goal/2)
)
tmpY1 *= -1;
if (Over(xdist2 +tmpX2/step, width/2-rest -obj2->get_radius()) ==
true && (xdist2+tmpX2/step >= goal/2 || xdist2+tmpX2/step <= -goal/2)
)
tmpX2 *= -1;
if (Over(ydist2 +tmpY2/step, length/2-rest -obj2->get_radius()) ==
true)
tmpY2 *= -1;
// check this FIRST because the first step could step over a
boundary first and this needs to check it before ?dist is set to set
the correct direction of tmp?

xdist1 = obj1->now_x() + i * (tmpX1/step); // where it is + i
fractions of the change, i.e. more of the fraction
ydist1 = obj1->now_y() + i * (tmpY1/step);

xdist2 = obj2->now_x() + i * (tmpX2/step);
ydist2 = obj2->now_y() + i * (tmpY2/step);

if ( sqrt( pow(xdist1 - xdist2, 2.0) + pow(ydist1 - ydist2, 2.0) ) <
obj1->get_radius()+obj2->get_radius() ) // distance between radii <
the sum of the radii
return i;
}

return 0;

}

int FixedCol(Movable* obj, double x, double y) { // returns 0 if there
no collision, else it returns the step of the process where the
collision was

double xdist = obj->now_x(); // its distance in the x direction after
any given number of steps through the process of deconstruction
double ydist = obj->now_y();

double tmpX = obj->change_x();
double tmpY = obj->change_y();

for (int i=1; i<=step; ++i)
{
if (Over(xdist +tmpX/step, width/2-rest -obj->get_radius()) == true)
// if any of the NEXT distances (thats why one step has been added)
are beyond a boundary, change the direction
tmpX *= -1;
if (Over(ydist +tmpY/step, length/2-rest -obj->get_radius()) == true
&& (xdist+tmpX/step >= goal/2 || xdist+tmpX/step <= -goal/2) )
tmpY *= -1;
// check this FIRST because the first step could step over a
boundary first and this needs to check it before ?dist is set to set
the correct direction of tmp?

xdist = obj->now_x() + i * (tmpX/step); // where it is + i fractions
of the change, i.e. more of the fraction
ydist = obj->now_y() + i * (tmpY/step);

if ( sqrt( pow(xdist - x, 2.0) + pow(ydist - y, 2.0) ) <
obj->get_radius() ) // distance between radii < the sum of the radii
return i;
}

return 0;

}

double GoalCoord(double coord, double fixed) {

if (coord > 0)
return fixed;
else
return -fixed;

}

double NewVel(double vel1, double vel2, double m1, double m2) {
return vel1 * (m1-m2) / (m1+m2) + vel2 * 2 * m2 / (m1+m2);
}

void HitTable(Movable* obj, double change) {

if (Over(obj->next_y(), length/2-rest) && obj->next_x() <=
goal/2-obj->get_radius() && obj->next_x() >=
-goal/2+obj->get_radius()) // GOAL !! only reset its position when its
beyond saving
{
obj->Replace();
return; // no need to continue
}

if ( Over(obj->next_x(), width/2-rest - obj->radius) ) // If its hit
the side walls
obj->x_change *= -change;

if ( Over(obj->next_y(), length/2-rest - obj->radius) ) // possibly
hit the front/back wall
{
if ( obj->next_x() >= goal/2 || obj->next_x() <= -goal/2 ) //
hitting the actual wall
obj->y_change *= -change;

else // hitting the corner or its a goal
{
double gx = GoalCoord(obj->next_x(), goal/2); // goal x
double gy = GoalCoord(obj->next_y(), length/2-rest); // goal y

if ( int stepped = FixedCol(obj, gx, gy) ) // hits the corner of
the goal because its distance from it is less than its radius
{
double xdist = obj->x + stepped * obj->x_change/step;
double ydist = obj->y + stepped * obj->y_change/step;

double x = ( -obj->y_change - xdist*(ydist-gy)/(xdist-gx) +
(xdist+obj->x_change)*(gx-xdist)/(ydist-gy) ) / (
(gx-xdist)/(ydist-gy) - (ydist-gy)/(xdist-gx) ); // effect of the
corner (vector)
double y = ydist + x*(ydist-gy)/(xdist-gx) -
xdist*(ydist-gy)/(xdist-gx); // y effect

double invx = (xdist+obj->x_change) - x; // inverse
(perpendicular) velocities calculated before points are made relative
- otherwise the geometry is broken
double invy = (ydist+obj->y_change) - y;

x -= xdist;
y -= ydist;

obj->x_change = invx + change * -x; // corner just reverses the
motion (very solid)
obj->y_change = invy + change * -y;
}
}
}

return;

}

void Collide() {

double energy = 0.9; // _energy_ is multiplied by the velocities
after a collision compensating for energy loss to sound, heat etc.

int stepped;

for (int a=0; a < MoveList.size(); ++a)
for (int b=a+1; b < MoveList.size(); ++b) // only go through the
ones you havent already, because the first went through all, the
second need only go from the 3RD onwards
{
if ( stepped = CircleCol(MoveList[a], MoveList[b]) )
{
double x1; // x velocity contributed via centre of mass, i.e. the
vector that contributes to the change, this is the x coordinate
double y1;

double x2;
double y2;

double invx1; // x velocity NOT contributed via centre of mass,
i.e. the vector that compliments x1 making up the velocity
double invy1;

double invx2;
double invy2;

double xdist1 = MoveList[a]->x +
stepped*MoveList[a]->x_change/step;
double ydist1 = MoveList[a]->y +
stepped*MoveList[a]->y_change/step;

double xdist2 = MoveList[b]->x +
stepped*MoveList[b]->x_change/step;
double ydist2 = MoveList[b]->y +
stepped*MoveList[b]->y_change/step;

if (xdist1 == xdist2) // prevent division by 0
{
x1 = 0;
y1 = MoveList[a]->y_change;

invx1 = 0;
invy1 = 0;

x2 = 0;
y2 = MoveList[b]->y_change;

invx2 = 0;
invy2 = 0;
}
else if (ydist1 == ydist2)
{
x1 = MoveList[a]->x_change;
y1 = 0;

invx1 = 0;
invy1 = 0;

x2 = MoveList[b]->x_change;
y2 = 0;

invx2 = 0;
invy2 = 0;
}
else
{
x1 = ( (xdist1 +
MoveList[a]->x_change)*((xdist1-xdist2)/(ydist2-ydist1)) -
MoveList[a]->y_change - xdist1*((ydist2-ydist1)/(xdist2-xdist1)) ) / (
(xdist1-xdist2)/(ydist2-ydist1) - (ydist2-ydist1)/(xdist2-xdist1) );
y1 = (x1*(ydist2-ydist1)/(xdist2-xdist1)) - xdist1 *
(ydist2-ydist1)/(xdist2-xdist1) + ydist1;

invx1 = (xdist1+MoveList[a]->x_change) - x1;
invy1 = (ydist1+MoveList[a]->y_change) - y1;

x2 = ( (xdist2 +
MoveList[b]->x_change)*((xdist2-xdist1)/(ydist1-ydist2)) -
MoveList[b]->y_change - xdist2*((ydist1-ydist2)/(xdist1-xdist2)) ) / (
(xdist2-xdist1)/(ydist1-ydist2) - (ydist1-ydist2)/(xdist1-xdist2) );
y2 = (x2*(ydist1-ydist2)/(xdist1-xdist2)) - xdist2 *
(ydist1-ydist2)/(xdist1-xdist2) + ydist2;

invx2 = (xdist2+MoveList[b]->x_change) - x2;
invy2 = (ydist2+MoveList[b]->y_change) - y2;

x1 -= xdist1; // its co-ordinates are only relative to its
position, lets give them an absolute value by making them be the
amount of change, making where they were useless
y1 -= ydist1;

x2 -= xdist2;
y2 -= ydist2;
}

if (MoveList[a]->controller() == physics) // only change its
velocity if its an object thats manipulated like that
{
MoveList[a]->x_change = invx1 + energy * NewVel(x1,x2,
MoveList[a]->get_mass(), MoveList[b]->get_mass()); // we want to take
its initial velocity and change it (according to the masses) by the
impact of the 2nd object
MoveList[a]->y_change = invy1 + energy * NewVel(y1,y2,
MoveList[a]->get_mass(), MoveList[b]->get_mass());

HitTable(MoveList[a], 0); // if the object has been hit by
another object and hits the wall as well, its going to be in a fast
rebound for many iterations, but physically, the mallet would absorb
the energy, thus passing 0.0 so the velocity is nothing
}

if (MoveList[b]->controller() == physics)
{
MoveList[b]->x_change = invx2 + energy * NewVel(x2,x1,
MoveList[b]->get_mass(), MoveList[a]->get_mass());
MoveList[b]->y_change = invy2 + energy * NewVel(y2,y1,
MoveList[b]->get_mass(), MoveList[a]->get_mass());

HitTable(MoveList[b], 0);
}
}
}

for (int i=0; i<MoveList.size(); ++i)
if (MoveList[i]->controller() == physics)
HitTable(MoveList[i], energy);

return;

}

void Reset() {

double leeway = 0.02; // amount that the reset function allows the
puck to be away from the wall, this converts to 0.5cm

for (int i=0; i < MoveList.size(); ++i)
if (MoveList[i]->controller() == physics) // a puck
if ( Over( MoveList[i]->next_x(), width/2-rest -
MoveList[i]->get_radius() - leeway ))
if ( Over( MoveList[i]->next_y(), length/2-rest -
MoveList[i]->get_radius() - leeway )) // only check your side, cant
reset on the other side
if ( sqrt( pow(MoveList[i]->change_x(), 2.0) +
pow(MoveList[i]->change_y(), 2.0) ) < leeway) // if total velocity is
almost nothing
MoveList[i]->Replace();

return;
}

void NextFrame() {

Reset();
Collide();

for (int i=0; i < MoveList.size(); ++i)
MoveList[i]->Move();

return;

}

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

[Victor Bazarov]

I am not subscribed to c.l.c++.m, so I'm only replying in c.l.c++...

"wired" <lu*****@hotmail.com> wrote...

I've just taught myself C++,
I hope you don't see it as an event. "I've just spilled a bowl
of soup on my trousers" or something like that...
so I haven't learnt much about style or
the like from any single source, and I'm quite styleless as a result.
It is expected. Having learned rules of chess one hasn't become
a player yet.
But at the same time, I really want nice code and I go to great
lengths to restructure my code just to look concise and make it more
manageable.
When I say this, I'm also referring to the way I write my functions.
It seems to me sometimes that I shouldn't have many void functions
accepting addresses and changing their values for example, but rather
a double function that returns the calculated value. i.e.
void Change(Type& variable);
should be replaced with
Type Change(Type variable);
Some do think that a function, since it's name that, should only
work like a mathematical namesake: give a value based on input
parameters. That would introduce too much limitation, and even
the standard library is not written that way. Keep that in mind.
I just don't know what's better code (in terms of speed, style, etc.).
Nobody knows. Style is a personal preference, speed is a feature
of a finished program. Yes, there are algorithms that serve the
same purpose and perform faster than others. Do use them if they
produce code the goal of which is as easy to recognise as with the
others. But don't concern yourself with speed too much before you
finish the functionality. First make it work, then make it fast.

You're right about making your code "maintainable" even if it means
maintenance by you. Some programmers produce enough code to easily
forget what they wrote a week before. They, like others would, go
back and look at their own code trying to figure out why certain
things were written in some way. So, for your own sake, do write
easily maintainable code.
I'm moving into a more mature phase of my program, and before I
continue I would like jsut a final verdict on my code.
Isn't this a bit too risky? I mean, using "final verdict" when
asking opinion on your code. If somebody says "it's crap", are
you going to give up programming? Hell, no. If somebody says,
"yeah, it'll work", are you going to stop improving yourself?
I'm looking for
comments about whether my functions should be more simple (i.e. too
much happens in each), whether I should rather return value instead of
alter through references passed, whether my general style is
inelegant. I'm also concerned about my class structures. But I'll show
you _some_ code and let you decide.

Please note that I'm not asking you to go through the actual logic of
all this code, but rather just calls to functions, classes and the
like.

Davin,

I chose not to comment on any particulars of your code. It is
partly due to my laziness, and due to the fact that there are too
many things I'd change. The first couple headers produced at
least a dozen lines I'd type and that suggested to me that you
have still ways to go and perhaps your venture into "better style"
by asking advice is a tad premature.

Start by getting and reading good books. "Effective C++" series,
"C++ FAQ", and just plain "Accelerated C++" and even "TC++PL" are
good sources of information about style if you know what to look
for. You're asking good questions, try finding answers to them
in those books first. Hell, simply read through the C++ FAQ Lite
(you can find it here: http://www.parashift.com/c++-faq-lite/).

Try not to believe everything you read (my reply to your post is
no exception). However, if more than one person tells you to do
something, try to at least make a note of that.

Do write more code. Only by writing you will develop your style.
And don't let anybody tell you what style to have, whatever you
will develop should be good enough, but it doesn't have to be
a copy of anything.

Good luck!

Victor

P.S. Couldn't resist. Just a couple of pointers to improve your
code dramatically and immediately. (a) Use initialisation of
class members, not assignment in constructors. (b) Use at least
some amount of indentation in your code, making it easier to
understand. (c) If an arithmetic expression tends to span more
than one line of code, wrap it into a function. (d) If there is
even a shadow of a possibility that your lines are too long and
are going to be wrapped [by a news reading software at posting],
use /**/ comments instead of //; don't let your lines to have more
length than an average screen can hold. (e) Use better news posting
software, Google is notoriously bad. (f) Put your declarations
in headers, definitions in source modules, your cpp files should
never have to contain 'extern' unless it's 'extern "C"'... What
else is noticeable at the first glance? "using" directives in
a namespace scope are dangerous... "friend" declarations can be
omitted by better design... There is no need for "return" at
the end of a void function... Conditions of "blah == true" are
too verbose, they should be "blah"... Nah, I better stop now or
I will never stop :-)

[John Harrison]

"wired" <lu*****@hotmail.com> wrote in message
news:a5**************************@posting.google.c om...

Hi,

[snip]

Victor's given you some good advice (with his usual brusque manner), here's
some comment on the actual code. Although as Victor said, don't believe
everything you read.

Please note that I'm not asking you to go through the actual logic of
all this code, but rather just calls to functions, classes and the
like.
First point is the total absence of the keyword const. There are a zilllion
places you should add this, obviously in your self teaching you didn't come
across this concept, you should make it the very next thing you learn about.

Thanks.

Davin.

//---------------------
//Globals.cpp - just some global variables needed by most functions
#include <vector>
#include "GLFiles.h"
#include "Derived.h"

using std::vector;

//quadratic to draw cylinders, spheres etc.
GLUquadricObj *quadratic;
I'm always doubtful about global variables.

//window dimensions
int WIDTH = 1024;
const int WIDTH = 1024;

Not going to point out every variable that you should be declaring const.
Although global variables are dubious, global constants are fine. Also if
you make a variable const, you should move it into a header file.
int HEIGHT = 768;

//table dimensions
float length = 25.15f; // 251.5cm
float width = 13.84f;
float height = 8.13f;
float goal = 4.0f;

float rest = 1.5f; // width between esdge and tabletop

// Misc

int step = 150; // number of steps the collision detection will
deconstruct each translation into in checking for a collision

Mallet* Mallet1;
Mallet* Mallet2;
vector <Puck*> Pucks;
More dubious globals, and I don't like pointers in STL classes, because it
makes memory management awkward. Since Puck is a polymorphic class I would
have used a smart pointer here. You can read about smart pointers in a good
book, 'More Effective C++' by Scott Meyers for instance.

double friction = 0.985;
int mouse_sens = 4; // 4-10, 10-least sensitive

//---------------------
//Abstract.h - two abstract classes, some derived classes will be
inherited from both
#ifndef Abstract_Classes
#define Abstract_Classes

#include "Globals.h"

using namespace std;

class Movable;
class Drawable;

void AddDraw(Drawable* draw);
void AddMove(Movable* move);

extern double friction;

class Drawable
{

public:

Drawable() {
AddDraw(this);
}

virtual void Draw() = 0;
As a classes which is designed for derivation Drawable must have a virtual
destructor other wise ou get undefined behaviour when you try to delete an
object of a class derived from Drawable using a pointer to Drawable.

virtual ~Drawable() {}

};

class Movable
{

public:

Movable(double x1, double y1, double r, Controller cont) {
Not sure what Controller is but consider a const reference here for
efficiency. And definitely prefer initialiation to assignment

Movable(double x1, double y1, double r, const Controller& cont) :
control(cont), x(x1), y(y1), radius(r), x_change(0.0), y_change(0.0) {

AddMove(this);

control = cont;

x = x1;
y = y1;

x_change = 0.0;
y_change = 0.0;

radius = r;

}
Ditto, virtual destructor needed.

virtual ~Movable() {}

double next_x() { return x + friction * x_change; }
Should be const as should most of the other methods.

double next_x() const { return x + friction * x_change; }
double next_y() { return y + friction * y_change; }

[snip]

OK, I'm bored to now. You do look like a self taught programmer. You've got
some concepts spot on, and others you've completely missed out on. You need
a good book, and practise.

john

[E. Robert Tisdale]

wired wrote:

It seems to me sometimes that I shouldn't have many void functions
accepting addresses and changing their values for example
but, rather, a double function that returns the calculated value. i.e.

void Modify(Type& variable);

should be replaced with

Type Transform(Type variable);
or

Type Transform(const Type& variable);
I just don't know what's better code (in terms of speed, style, etc.).
Sometimes, you must modify objects "in-place" so you would implement

Type& Modify(Type& variable);

where the function modifies variable in-place then returns a reference
to variable so that your Modify function can be used in expressions
that may also modify variable. For example,
an object which is an instance of one of the container classes
is almost always a variable which requires modification.

Otherwise, whether you pass function arguments
by value or by const reference usually depends
only upon the size of the object.
If the object occupies no more than one or two machine words,
it is probably faster and more efficient to pass by value.
Larger objects are almost always passed by const reference.
cat f.h #include<iostream>

class X {
private:
// representation
int I;
public:
// operators
operator int(void) const { return I; }
X(int i): I(i) { }
};

X f(X);
X f(const X&);
cat f.cc #include<f.h>

X f(X x) {
std::cout << "X f(X x)" << std::endl;
return (int)x + 33;
}

X f(const X& x) {
std::cout << "X f(const X& x)" << std::endl;
return (int)x + 33;
}
cat main.cc #include<f.h>

int main(int argc, char* argv[]) {
X x = 22;
X y = f(x);
std::cout << (int)y << " = f(x)" << std::endl;
return 0;
}
g++ -Wall -ansi -pedantic -I. -O2 -o main main.cc f.cc

main.cc: In function `int main(int, char**)':
main.cc:5: call of overloaded `f(X&)' is ambiguous
f.h:13: candidates are: X f(X)
f.h:14: X f(const X&)

The nice thing about this is that you can substitute
function f(const X&) for function f(X) and vice versa
without affecting the meaning of the application program
just as long as function f doesn't modify any global variable
that can be passed to it in its argument list.
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

[wired]

Hi,

Thanks heaps! The response has been fantastic, and the advice endless
(for good reason). Sorry about the lack of indentation, in my code its
there, and through the copy-paste it must have squeezed itself - as
suggested I should think about a proper newsreader.

Its good to get back consistent feedback (const use, globals,
namespaces etc.), incredibly settling!

So once again thanks for your help.

Any further posts I'll be sure to read though, so don't stop posting
because I posted this message :-)

Davin.

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

[Francis Glassborow]

In message <3E**************@jpl.nasa.gov>, E. Robert Tisdale
<E.**************@jpl.nasa.gov> writes

1) Functions that are designed for doing (e.g. display a graphic)
should return void or, if member functions possibly *this.

Nonsense!
There is seldom a good reason to define a void function.
(A destructor, for example, is an exception to this rule.)
At the very least, a function should return a reference
(or, perhaps, a pointer) to the object that was modified
so that the function can be used in expressions.

You are entitled to an opinion but it is not widely shared by expert C++
programmers (such as Bjarne Stroustrup, Nico Josuttis, Scott Meyers, Rob
Murray, Steve Dewhurst, Herb Sutter, Andrei Alexandrescu ...). The
significance of a function returning void in C++ is that it is really a
procedure which is a well understood concept in computer science.
And destructors simply do not have return values of any kind so they
are not exceptions to a guideline, the programmer is never given the
choice.

What should be the return type of:

??? foo(){
std::cout << std::clock();
}

--
ACCU Spring Conference 2003 April 2-5
The Conference you should not have missed
ACCU Spring Conference 2004 Late April
Francis Glassborow ACCU
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

[kanze]

Francis Glassborow <fr****************@ntlworld.com> wrote in message
news:<hJ**************@robinton.demon.co.uk>...

In message <a5**************************@posting.google.com >, wired
<lu*****@hotmail.com> writes

Please note that I'm not asking you to go through the actual logic of
all this code, but rather just calls to functions, classes and the
like.

A few possible guidelines:
All excellent. Just one further comment...
4) Think very carefully about deriving from a concrete class 5) Think even more carefully about multiple inheritance from concrete
classes. Mostly MI works well for adding interfaces.

I think that these two rules can be subsumed by a more general rule:
think in terms of patterns, and what pattern(s) your inheritance
hierarchy uses. IMHO, there is nothing per se really wrong about
deriving from a concrete class, even with multiple inhertance. It's
just that there are remarkably few patterns that do it, so that most of
the time, it tends to be a symptom of class design by carelessly
throwing various functionality together.

--
James Kanze GABI Software mailto:ka***@gabi-soft.fr
Conseils en informatique orientée objet/ http://www.gabi-soft.fr
Beratung in objektorientierter Datenverarbeitung
11 rue de Rambouillet, 78460 Chevreuse, France, Tél. : [phone]

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

[Thomas Mang]

"E. Robert Tisdale" schrieb:

Otherwise, whether you pass function arguments
by value or by const reference usually depends
only upon the size of the object.
If the object occupies no more than one or two machine words,
it is probably faster and more efficient to pass by value.
Larger objects are almost always passed by const reference.

Really? That's news to me.

Here is a very simple String class, with size most likely to be 4 on a 32
bit machine:

class String
{
public:
String(const char* Text) : text_(new char[std::strlen(Text) + 1])
{std::strcpy(text_, Text);}
~String() {delete [] text_;}
String(const String& rhs) : text_(new char[std::strlen(rhs.text_) + 1])
{std::strcpy(text_, rhs.text_);}
// assignment operator, .....

private:
char* text_;
};

Would you pass such a class around by value?
I wouldn't, because the copy constructor is expensive.

OTOH, larger classes with a cheap copy constructor (e.g. one that copies its
members by bitwise copying) won't hurt much when passed-by-value.

Just looking at what sizeof() returns and deciding on that number whether to
pass-by-value or -by-reference is, at least IMHO, not the way to go.
regards,

Thomas

regards,

Thomas

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

[Victor Bazarov]

"Francis Glassborow" <fr****************@ntlworld.com> wrote...

In message <3E**************@jpl.nasa.gov>, E. Robert Tisdale
<E.**************@jpl.nasa.gov> writes

1) Functions that are designed for doing (e.g. display a graphic)
should return void or, if member functions possibly *this.
Nonsense!
There is seldom a good reason to define a void function.
(A destructor, for example, is an exception to this rule.)
At the very least, a function should return a reference
(or, perhaps, a pointer) to the object that was modified
so that the function can be used in expressions.

You are entitled to an opinion but it is not widely shared by expert

C++ programmers (such as Bjarne Stroustrup, Nico Josuttis, Scott Meyers, Rob Murray, Steve Dewhurst, Herb Sutter, Andrei Alexandrescu ...). The
significance of a function returning void in C++ is that it is really a procedure which is a well understood concept in computer science.
And destructors simply do not have return values of any kind so they
are not exceptions to a guideline, the programmer is never given the
choice.

What should be the return type of:

??? foo(){
std::cout << std::clock();
}

'bool' or 'iostream&' so that the caller could verify the completeness
of the operation. The code should probably have

return std::cout.good();

or

return std::cout;

in it.

But that's more or less irrelevant. Your argument about 'void'
functions' right to exist is valid. You just didn't use a good
example.

Victor

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

[Jerry Coffin]

In article <a5**************************@posting.google.com >, lunz008
@hotmail.com says...

[ ... ]

A few more comments on things I haven't seen anybody else mention yet.

//Abstract.h - two abstract classes, some derived classes will be
inherited from both
#ifndef Abstract_Classes
#define Abstract_Classes

#include "Globals.h"

using namespace std;
A using declaration like this in a header should normally be avoided.

[ ... ]
class Movable
{ [ ... ]
double get_radius() { return radius; }
double get_mass() { return mass; }
So everything that's movable has a radius and a mass? That sounds a
little questionable to me...

[ ... ]
class Mallet: public Drawable, public Movable [ ... ] class Puck: public Drawable, public Movable
Are there objects that are drawable and not movable, or vice versa?
Right now, it looks like all actual objects are both drawable and
movable, in which case putting those into separate classes accomplishes
nothing useful.

In Mallet::Move, you have an explicit check for whether the controller
is the AIController, or the MouseController, and then you get the
coordinates from one or the other. This is almost certainly a poor
idea. I'd use something like:

Controller {
public:
virtual GeoMotion getCoords() = 0;
};

class MouseCont : public Controller {
public:
GeoMotion getCoords {
// ...
}
};

class AICont : public Controller {
public:
GeoMotion getCoords {
// ...
}
};

Then void Mallet::Move() {

GeoMotion coords(this);

if (control == mouse)
coords = MouseCont(this);
else if (control == AI)
coords = AICont(this);
becomes:

void Mallet::Move() {

GeoMotion coords(this);

cords = Controller.getCoords();
x = coords.x;
y = coords.y;
If you're going to define a coords class, I'd try to use it wherever
applicable. E.g.:

coords current_pos;

in which case, you'd just use:

current_pos = Controller.getCoords();
x_change = coords.x_change;
y_change = coords.y_change;
It seems highly suspect to me to have something called "coords" that
apparently only specifies coordinates, but also a movement vector.
if (y<0) // if it needs to be reset on the other side
y = -length/4;
else
y = length/4;
y = fabs(length/4);

[ ... ]
x >= 0? x=-x-radius/2 : x=-x;
This isn't really how the conditional is intended to be used. The
intent is more like:

x = x>=0 ? x-radius/2 : -x;

[ ... ]
} while (overlap == true);
As a rule, comparing anything to true is a poor idea. I'd just use:

} while (overlap);

[ ... ]
if (LoadPucks(1) != true || LoadMallets() != true) // LoadPucks first
Likewise, here I'd just use:
if ( !loadPucks(1) || !LoadMallets())
try {
tmpPuck = new Puck(0,5);
Pucks.push_back(tmpPuck);
}
catch(std::bad_alloc nomem)
{
return false;
}
IMO, this is a poor idea. It would probably be better to use exception
handling throughout, so this part of the code would just let the
exception propagate to somewhere else that it could really be handled.

[ ... ]
bool Over(double coord, double fixed) { // has coord gone beyond fixed
in both positive and negative dimensions

if (coord > fixed || coord < -fixed)
return true;

return false;
return coord > fixed || coord < - fixed;
void Collide() {

double energy = 0.9; // _energy_ is multiplied by the velocities
after a collision compensating for energy loss to sound, heat etc.

IMO, most things like this should normally be in external data files.
This makes it much easier to do things like defining objects with
different characteristics (e.g. hard pucks vs. soft, squishy pucks).

I'd also define this to receive a couple of iterators defining the
collisions to check, rather than always operating on the global
MoveList.

On a more general level, your code is composed mostly of a few large
classes and a few large functions. I'd try to break things up a bit
more, into discrete chunks that are smaller and easier to understand
individually. Just for a really obvious example, you have the usual
Pythagorean distance formula in a LOT of different places. The code
would be much easier to understand if you defined a function for this,
and then used it elsewhere. In addition, a few temporary variables can
help readability dramatically. eg.:

float dist(int x, int y) {
return sqrt(x * x + y * y);
}

// If I'm following this correctly, this loop can be replaced
// with some geometry and end up faster and more accurate.
void find_step(GeoMotion &c, Moveable const &m) {
for (int i=0; i<step; ++i) {
float x_step = c.x+i*c.x_change/step;
float y_step = c.y+i+c.y_change/step;

if (dist(x_step, y_step) < c.radius + m.get_radius()) {
c.x_change = (j-1)*c.x_change/step;
x.y_change = (j-1)*c.y_change/step;
// guessing at an addition:
break;
}
}
}

void MalletSquash(GeoMotion &coords, Mallet *mal) {
for (int i=0; i<MoveList.size(); ++i) {
Moveable &m = *MoveList[i];
if ( &m != mal && m.controller() == physics) {
double next_x = coords.x+coords.x_change;
double next_y = coords.y+coords.y_change;
double &r = coords.radius;

if ( dist(next_x, next_y) < r + m.get_radius())
if (Over(next_x, ...) || Over(next_y, ...)
find_step(coords, m);
}
}
}

and we're at least getting a bit closer to something readable. I doubt
it's optimal yet, but I'm not sure I followed what was going on well
enough to suggest many further improvements.

--
Later,
Jerry.

The universe is a figment of its own imagination.

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

[E. Robert Tisdale]

Francis Glassborow wrote:

In message <3E**************@jpl.nasa.gov>, E. Robert Tisdale
<E.**************@jpl.nasa.gov> writes

1) Functions that are designed for doing (e.g. display a graphic)
should return void or, if member functions possibly *this.
Nonsense!
There is seldom a good reason to define a void function.
(A destructor, for example, is an exception to this rule.)
At the very least, a function should return a reference
(or, perhaps, a pointer) to the object that was modified
so that the function can be used in expressions.

You are entitled to an opinion
but it is not widely shared by expert C++ programmers
(such as Bjarne Stroustrup, Nico Josuttis, Scott Meyers, Rob Murray,
Steve Dewhurst, Herb Sutter, Andrei Alexandrescu ...).

Please cite the references and quote the relevant passages.
The significance of a function returning void in C++ is that
it is really a procedure
which is a well understood concept in computer science.
It is well understood that procedural programs
are difficult read, understand and maintain.
Each procedure call changes the state of the program.
The state affects the flow control
which can change the thread of execution.
The thread of execution determines the sequence of procedure calls, etc.
What you end up with is spaghetti.

Procedural programs are difficult to analyze
and practically impossible to prove correct.
You should avoid procedure calls (and assignment statements)
whenever possible.
And destructors simply do not have return values of any kind
so they are not exceptions to a guideline,
the programmer is never given the choice. What should be the return type of:

std::ostream& foo(std::ostream& os){
std::os << std::clock();
return os;
}
At the very least, a function should return a reference
to the object that was modified
so that the function can be used in expressions.
cat Francis.cc #include<iostream>

std::ostream& foo(std::ostream& os){
os << std::clock();
return os;
}

int
main(int argc, char* argv[]) {
foo(std::cout) << std:: endl;
return 0;
}
g++ -Wall -ansi -pedantic -O2 -o Francis Francis.cc

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

[Ed Avis]

Francis Glassborow <fr****************@ntlworld.com> writes:

There is seldom a good reason to define a void function. (A
destructor, for example, is an exception to this rule.) At the very
least, a function should return a reference (or, perhaps, a pointer)
to the object that was modified so that the function can be used in
expressions.
You are entitled to an opinion but it is not widely shared by expert
C++ programmers (such as Bjarne Stroustrup, Nico Josuttis, Scott
Meyers, Rob Murray, Steve Dewhurst, Herb Sutter, Andrei Alexandrescu
...).

You can't just drop a long list of names without giving some
references to where we can find their opinions to back you up. And if
giving references for each of them would be too long-winded, just
enlist fewer of them in your support.

I imagine that the behaviour of C++'s assignment operator - which
returns a reference to the object that was modified, just as the
previous poster suggested - indicates that B. Stroustrup prefers that
style rather than the 'pure procedural' style where modification
operations cannot be changed.
The significance of a function returning void in C++ is that it is
really a procedure which is a well understood concept in computer
science.
What exactly is this concept? If you say that 'a procedure does not
return a value' then that is tautologically true, but it doesn't give
any reason to write programs in this style.
What should be the return type of:

??? foo(){
std::cout << std::clock();
}

This doesn't fit the case proposed for having a returned value, which
is when the value of an object is modified. operator<<() isn't really
a value-modification operation.

Still, you might like to consider what is the return type of
operator()<< itself.

--
Ed Avis <[e-mail]>

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

[Sean Fraley]

E. Robert Tisdale wrote:

> The significance of a function returning void in C++ is that
> it is really a procedure
> which is a well understood concept in computer science.
It is well understood that procedural programs
are difficult read, understand and maintain.
Each procedure call changes the state of the program.
The state affects the flow control
which can change the thread of execution.
The thread of execution determines the sequence of procedure calls, etc.
What you end up with is spaghetti.

He didn't say that a "prodedural program" is a well understood concept in
computer science. He said that a "procedure" is a well understood concept
in computer science. He is talking about a logical concept, no an example
of a particular design paradigm.
Procedural programs are difficult to analyze
and practically impossible to prove correct.
The problem may lay with the skills of the person doing the analysis, not
the fact that the program is procedural.
You should avoid procedure calls (and assignment statements)
whenever possible.

Why? I can possibly see this from some academically pure point of view, but
in the real world, writing real programs that are used to solve real
problems, this statement seems rather inane. If you're going to make a
broad doctrinal statement like "avoid procedural calls whenever possible",
then at least provide some rationale as to why. The person who started
this thread is someone who is looking for input and ideas to help them
progress as a programmer. We want up and coming programmers to know WHY
something is done, so that way they can make an informed decision as to
WHAT they should do.

> And destructors simply do not have return values of any kind
> so they are not exceptions to a guideline,
> the programmer is never given the choice.

> What should be the return type of:
>
> std::ostream& foo(std::ostream& os){
> std::os << std::clock();
> return os;
> }

At the very least, a function should return a reference
to the object that was modified
so that the function can be used in expressions.

Once again, why? In the example that you give above, you've not really
accomplished anything useful. The operator<< functions of std::ostream
already return a reference to the ostream. If I want to insert the return
value of std::clock into an ostream, then

std::cout << "this program has run for " << std::clock()
<< " clock ticks.\n";

is actually a better choice than

std::cout << "this program has run for "
<< foo(std::cout)
<< "clock ticks.\n";

for the following reasons:

1) it works like you think it should. Try writing, compiling, and then
running some code with your implementation of foo. It does some weird
stuff.

2) you don't have to pass a parameter to foo, thereby reducing the
requirements of code that is a client of foo.

3) the original implementations purpose is immediately apparent at even a
slight glance, and a client with access to nothing but the prototype in the
header sees nothing confusing about it. With you're implementation, the
first question to be asked by a client is "why the f*** should I have to
pass an ostream as a parameter just to print the number of clock ticks
since start to standard output? That makes no sense."

he original implementation makes since in terms of the design of the
program. If the specification says "a function to print the number of
clock ticks since start to standard output." Then a void function that
takes no arguments is just what you need.

This is why I have a real problem with purely academic doctrine being thrown
around like it is some kind of time tested wisdom. In order to change

void foo()
{
std::cout << std::clock;
}

to fit into your little rule of "always return something, even if it's a
reference to the object being modified.", you had to remove most of what
made it useful. The original implementation makes since in terms of the
design of the program. If the specification says "a function to print the
number of clock ticks since start to standard output." Then a void
function that takes no arguments is just what you need. Drastically
changing it's argument list, not to mention implementing it in such a way
that it behaves badly at runtime, for no other reason than the slim chance
that someone might want to use it in an expression at some point in the
future, is a bad decision.

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

[Francis Glassborow]

In message <l1************@budvar.future-i.net>, Ed Avis
<[e-mail]> writes

Francis Glassborow <fr****************@ntlworld.com> writes:

There is seldom a good reason to define a void function. (A
destructor, for example, is an exception to this rule.) At the very
least, a function should return a reference (or, perhaps, a pointer)
to the object that was modified so that the function can be used in
expressions.
You are entitled to an opinion but it is not widely shared by expert
C++ programmers (such as Bjarne Stroustrup, Nico Josuttis, Scott
Meyers, Rob Murray, Steve Dewhurst, Herb Sutter, Andrei Alexandrescu
...).

You can't just drop a long list of names without giving some
references to where we can find their opinions to back you up. And if
giving references for each of them would be too long-winded, just
enlist fewer of them in your support.

You miss the point. Please name a single respectable C++ writer who
follows Mr Tisdale's dictum.

I imagine that the behaviour of C++'s assignment operator - which
returns a reference to the object that was modified, just as the
previous poster suggested - indicates that B. Stroustrup prefers that
style rather than the 'pure procedural' style where modification
operations cannot be changed.

Let me reinsert the context:

1) Functions that are designed for doing (e.g. display a graphic) should
return void or, if member functions possibly *this.

Which was one of my guidelines wrt to writing code. Now go and examine
the writings of any of the above list of authors and you will find that
they implicitly apply that guideline in their published code (i.e. you
will find them using a void return type whenever there is no sensible
value to return.

Bjarne Stroustrup was/is following a policy of as close to C... so he
had very little choice about operators including the assignment ones in

C++ yielding values. I suspect that Ritchie may have been heavily
influenced by the precursors to C (such as B and BCPL). In practice
there are numerous bugs in code that are a result of the C and C++
assignment operators returning values.
And where did I mention a pure procedural style? I would like some
support for a pure functions but that is a different issue.

It is also interesting to note that relatively late on C++ increased its

support for 'procedure' type functions by allowing return of the call of

a void function to a function with a void return.

--
ACCU Spring Conference 2003 April 2-5
The Conference you should not have missed
ACCU Spring Conference 2004 Late April
Francis Glassborow ACCU
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

[John Potter]

On 1 Jul 2003 14:21:43 -0400, Sean Fraley <sf*****@rhiannonweb.com>
wrote:

std::cout << "this program has run for " << std::clock()
<< " clock ticks.\n"; is actually a better choice than std::cout << "this program has run for "
<< foo(std::cout)
<< "clock ticks.\n"; for the following reasons: 1) it works like you think it should. Try writing, compiling, and then
running some code with your implementation of foo. It does some weird
stuff.

Be nice. Do not misuse it. Use it properly and complain about that.

foo(std::cout << "this program has run for ")
<< "clock ticks.\n";

John

[E. Robert Tisdale]

John Potter wrote:

Sean Fraley wrote:

std::cout << "this program has run for " << std::clock()
<< " clock ticks.\n";

is actually a better choice than

std::cout << "this program has run for "
<< foo(std::cout)
<< "clock ticks.\n";

for the following reasons:

1) it works like you think it should. Try writing, compiling,
and then running some code with your implementation of foo.
It does some weird stuff.

Be nice. Do not misuse it. Use it properly and complain about that.

foo(std::cout << "this program has run for ")
<< "clock ticks." << std::endl;

I hope everyone remembers that Sean is complaining about
Francis Glassborow's function and not mine.
I felt that I was obliged to do the best I could
with the function that Francis gave us.

[E. Robert Tisdale]

Francis Glassborow wrote:

Let me reinsert the context:

1) Functions that are designed for doing (e.g. display a graphic) should
return void or, if member functions possibly *this.

Which was one of my guidelines wrt to writing code. Now go and examine
the writings of any of the above list of authors and you will find that
they implicitly apply that guideline in their published code (i.e. you
will find them using a void return type whenever there is no sensible
value to return.

Bjarne Stroustrup was/is following a policy of as close to C... so he
had very little choice about operators including the assignment ones in

C++ yielding values. I suspect that Ritchie may have been heavily
influenced by the precursors to C (such as B and BCPL). In practice
there are numerous bugs in code that are a result of the C and C++
assignment operators returning values.
And where did I mention a pure procedural style? I would like some
support for a pure functions but that is a different issue.

It is also interesting to note that relatively late on C++ increased its

support for 'procedure' type functions by allowing return of the call of

a void function to a function with a void return.

According to Brian W. Kernighan and Dennis M. Ritchie,
"The C Programming Language", Copyright 1978,
Chapter 6 Structures, Section 2 Structures and Functions, page 121:

"There are a number of restrictions on C structures.
The essential rules are that the only operations
that you can perform on a structure are take its address with &,
and access on of its members. This implies that
structures may not be assigned to or copied as a unit,
and that they can not be passed to or returned from functions.
(These restrictions will be removed in forth-coming versions.)"

Appendix A C Reference Manual, Section 14 Types revisited,
Subsection 1 Structures and unions, page 209:

"There are only two things that can be done with structures and unions:
name one of its members (by means of the . operator);
or take its address (by unary &). Other operations, such as assigning
from or to it or passing it as a parameter, draw an error message."

These restrictions on passing and returning struct's
have long since been removed from the C computer programming language
but old habits die hard -- especially bad habits.
There is no longer any reason to pass a pointer to the return value
explicitly in the function argument list.
Your compiler will do that for you.
It is still a good idea to pass a const pointer
if you need to reference large objects from the function.

[Albrecht Fritzsche]

Ed Avis wrote:

You are entitled to an opinion but it is not widely shared by expert
C++ programmers (such as Bjarne Stroustrup, Nico Josuttis, Scott
Meyers, Rob Murray, Steve Dewhurst, Herb Sutter, Andrei Alexandrescu
...).

You can't just drop a long list of names without giving some
references to where we can find their opinions to back you up.

It is really not that hard to find some articles by those mentioned
above. Google sometimes does wonders, but

www.cuj.com/experts

might be a sufficient starting point if you haven't have any books
by those authors handy or not read yet.
What exactly is this concept? If you say that 'a procedure does not
return a value' then ... it doesn't give
any reason to write programs in this style.

Of course not, but if you head for a clear and concise(?) style than
you might find procedures sometimes the most valuable tool. Just
have a look at the STL - there those procedures are called algorithms
and more often than not having a void return.

Until now noone has complained, though.
Ali
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

[John Potter]

On 1 Jul 2003 15:34:11 -0400, Francis Glassborow
<fr****************@ntlworld.com> wrote:

1) Functions that are designed for doing (e.g. display a graphic) should
return void or, if member functions possibly *this.
ostream& operator<< (ostream&, string const&);
iterator vector<T>::insert(iterator, T const&);
Which was one of my guidelines wrt to writing code. Now go and examine
the writings of any of the above list of authors and you will find that
they implicitly apply that guideline in their published code (i.e. you
will find them using a void return type whenever there is no sensible
value to return.

Your statement is acceptable, but not your intent. Whenever there is
any way to use the imagination to create a sensible return, the function
is not void. Void is a last resort used in inverse proportion to the
sensibleness of the user. :)

John

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

[Francis Glassborow]

In message <3F**************@jpl.nasa.gov>, E. Robert Tisdale
<E.**************@jpl.nasa.gov> writes

You are entitled to an opinion
but it is not widely shared by expert C++ programmers
(such as Bjarne Stroustrup, Nico Josuttis, Scott Meyers, Rob Murray,
Steve Dewhurst, Herb Sutter, Andrei Alexandrescu ...).
Please cite the references and quote the relevant passages.

Please read clause 25, Algorithms library of the C++ standard. It is too
long to quote here but you should find a copy in JPL's reference
library.
Those wishing to avoid using procedures (functions returning nothing)
must, effectively, avoid the C++ library and any third party library
that does not guarantee to avoid all standard functions that return
void.

The significance of a function returning void in C++ is that
it is really a procedure
which is a well understood concept in computer science.
It is well understood that procedural programs
are difficult read, understand and maintain.

I was not writing about procedural programs but that certain actions are
inherently procedures (i.e. are used for their side effects). Even
functional programming languages such as Haskell accept that some
procedural type actions are unavoidable though they try to minimise
them.
Each procedure call changes the state of the program.
But that is just a subset, each call of a function that has side effects
potentially changes the state of the program. A function in C++ can only
return one thing, which means that any function that is (in)directly
responsible for changing two or more non-local objects has to choose
which to return if either.
The state affects the flow control
which can change the thread of execution.
The thread of execution determines the sequence of procedure calls, etc.
What you end up with is spaghetti.
If we try to use uncontrolled procedural programming, but that is very
different from using procedures to carry out actions and functions to
evaluate and return values. Unfortunately (in the opinion of purists)
C++ supports everything from pure procedures to pure functions with
every conceivable mixture in between. At least I can call out my
procedures by giving them a void return type. I wish I had a similar
mechanism to identify pure functions.

Procedural programs are difficult to analyze
and practically impossible to prove correct.
I never advocated writing pure procedural programs.
You should avoid procedure calls (and assignment statements)
whenever possible.
Then C++ is a bad language to use, Haskell would be a much better
choice.

And destructors simply do not have return values of any kind
so they are not exceptions to a guideline,
the programmer is never given the choice.

What should be the return type of:

std::ostream& foo(std::ostream& os){
std::os << std::clock();
return os;
}

That is not what I wrote, please be more careful about attributions. Had
I wanted such a function I would have written:

std::ostream& foo(std::ostream & out){
return out<< std::clock();
}

or even:

bool foo(std::ostream & out)
return not (out << std::clock()).fail();
}

Please note that your code includes a non-existent object (std::os) and
so should fail to compile.
--
ACCU Spring Conference 2003 April 2-5
The Conference you should not have missed
ACCU Spring Conference 2004 Late April
Francis Glassborow ACCU
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

[Tony]

I am not C++ expert in any way.
I like the combination of procedural style and oo style in C++. There
are many things which best be treated in oo style. There are just a
few things which bested be treated in procedual style.
Take y = sin(x). This calculation in my view best be treated as is
like in math, in procedural style. C++ gives us both styles, I think
it is a good thing.
In Java, y = sin(x) is a member of Math class. I found Java way is not
natural.
Tony

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

[kanze]

John Potter <jp*****@falcon.lhup.edu> wrote in message
news:<qf********************************@4ax.com>. ..

On 1 Jul 2003 15:34:11 -0400, Francis Glassborow
<fr****************@ntlworld.com> wrote:

> 1) Functions that are designed for doing (e.g. display a graphic)
> should return void or, if member functions possibly *this. ostream& operator<< (ostream&, string const&);
iterator vector<T>::insert(iterator, T const&);
void vector<T>::insert(iterator, size_type, T const&);

I don't think you can quote the C++ library for examples of good design.
There are a few: separating the formatting from the sinking/sourcing in
iostream, for example. But on the whole... Even within a single class,
it can't be consistent.
> Which was one of my guidelines wrt to writing code. Now go and
> examine the writings of any of the above list of authors and you
> will find that they implicitly apply that guideline in their
> published code (i.e. you will find them using a void return type
> whenever there is no sensible value to return.

Your statement is acceptable, but not your intent. Whenever there is
any way to use the imagination to create a sensible return, the
function is not void. Void is a last resort used in inverse
proportion to the sensibleness of the user. :)

If you have to use your imagination or your creativeness to invent a
return value, you probably would be better off with void. What should a
function like EventGenerate::enrolForEvent( EventHandler& ) return, for
example? Or the callback EventHandler::operator()( Event const& ) ?

Generally speaking, if the purpose of a function is to obtain a value,
then that value should be returned by means of the return of the
function. Otherwise, if the function can fail in an unexceptional way,
the return value should be the error code. Otherwise, in general, the
return value should be void -- there are a few cases where supporting
chaining imposes some other return value, but they aren't that often.

--
James Kanze GABI Software mailto:ka***@gabi-soft.fr
Conseils en informatique orientée objet/ http://www.gabi-soft.fr
Beratung in objektorientierter Datenverarbeitung
11 rue de Rambouillet, 78460 Chevreuse, France, Tél. : [phone]

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

[kanze]

Sean Fraley <sf*****@rhiannonweb.com> wrote in message
news:<LK**********************@twister.neo.rr.com> ...

E. Robert Tisdale wrote:

> What should be the return type of: > std::ostream& foo(std::ostream& os){
> std::os << std::clock();
> return os;
> }

At the very least, a function should return a reference to the
object that was modified so that the function can be used in
expressions.

Once again, why?

In that case, one could just as easily ask: why not? The fact that some
forms of std::vector::erase return iterator, and others return void, is
obviously poor design.

But that begs the point. The fact that there are times when some
"procedures" should logically return a value (and thus become functions)
doesn't mean that all should. The initial argument was that using
"procedures" makes reasoning about the program impossible, and results
in spaghetti code. Just returning some random value, which might
eventually permit chaining, won't make the program easier to
understand. In fact, I'd argue just the opposite -- if the statement
contains just one procedure invocation, and nothing else, I know that it
does exactly what that procedure does. Once you start chaining
functions with side effects (i.e. procedures with return values), you're
making things even more complex and difficult to understand. This
argument actually weighs in in favor of what Francis originally stated:
if the function isn't really a function, then don't fool the reader by
making it look like one. (The words are mine, but I think they more or
less correspond with what Francis meant.) I would qualify it somewhat:
there are procedures which can "fail" in expected (non-exceptional)
ways, for which using a return value to indicate error state would be
appropriate.

And in real life, we do need procedures. If I call a procedure to write
something to a file, or to display something on the screen, it had
better have some side effects.

--
James Kanze GABI Software mailto:ka***@gabi-soft.fr
Conseils en informatique orientée objet/ http://www.gabi-soft.fr
Beratung in objektorientierter Datenverarbeitung
11 rue de Rambouillet, 78460 Chevreuse, France, Tél. : [phone]

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

[John Potter]

On 1 Jul 2003 22:48:02 -0400, Albrecht Fritzsche
<al****************@tiscali-dsl.de> wrote:

Of course not, but if you head for a clear and concise(?) style than
you might find procedures sometimes the most valuable tool. Just
have a look at the STL - there those procedures are called algorithms
and more often than not having a void return.

You seem to count funny. I get void returns on 30 of 102 algorithms.
Existence: yes, majority: no. Sorting accounts for 18 of the 30.

John

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

[E. Robert Tisdale]

Ed Avis wrote:

Displaying a graphic doesn't really 'modify' that graphic,
so I agree, it should return void.

It modifies the display.
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

[Francis Glassborow]

In message <d6*************************@posting.google.com> ,
ka***@gabi-soft.fr writes

This
argument actually weighs in in favor of what Francis originally stated:
if the function isn't really a function, then don't fool the reader by
making it look like one. (The words are mine, but I think they more or
less correspond with what Francis meant.) I would qualify it somewhat:
there are procedures which can "fail" in expected (non-exceptional)
ways, for which using a return value to indicate error state would be
appropriate.

Yes that is much my view. If a procedure can fail in ways that influence
the rest of the program I normally give it a return type of bool. Note
that that has nothing to do with whatever the procedure was doing, it is
just an effective way of reporting failure.

Indeed only yesterday I wrote some code that went like this:

bool dosomething(mytype & mt){
try {
// do various things that modify mt
// but always leave mt stable
return true;
}
catch(...) {return false;}
}

In the circumstances I did not care what had gone wrong, just that it
had not worked as intended. Of course this function is also terrible
because it uses SEME :-)
--
ACCU Spring Conference 2003 April 2-5
The Conference you should not have missed
ACCU Spring Conference 2004 Late April
Francis Glassborow ACCU
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

[Francis Glassborow]

In message <3F**************@jpl.nasa.gov>, E. Robert Tisdale
<E.**************@jpl.nasa.gov> writes

Ed Avis wrote:

Displaying a graphic doesn't really 'modify' that graphic,
so I agree, it should return void.

It modifies the display.

And if that is done by a system call? What should I then return?

--
ACCU Spring Conference 2003 April 2-5
The Conference you should not have missed
ACCU Spring Conference 2004 Late April
Francis Glassborow ACCU
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

[Ed Avis]

br******@cix.co.uk (Dave Harris) writes:

And you should make sure no-one confuses a procedure with
a function. One way to achieve that is to make sure functions never
have side effects and procedures never return results.

I fear this may turn into another SESE-ish discussion. While such a
style is often useful, I wouldn't like to mandate it for all code, all
of the time. Inherently stateful things like an I/O library don't
really need to follow the rule, since it is obvious that
print_string() has a side effect, whether or not it returns void.

It would help if compilers had some way for the programmer to assert
that a function has no side effects, and have this statically
checked. I think gcc has some weird attribute you can set on a
function to say it is pure and so calls to it may be optimized away,
but it's the programmer's responsibility to make sure the annotation
is sound.

--
Ed Avis <[e-mail]>

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

[E. Robert Tisdale]

Francis Glassborow wrote:

And you were responding to Mr. Tisdale's rewriting of what I had written.

Your function

void foo(void){
std::cout << std::clock();
}

modifies a global variable -- cout.
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

[Rob]

"Francis Glassborow" <fr****************@ntlworld.com> wrote in message
news:y6**************@robinton.demon.co.uk...

In message <3F**************@jpl.nasa.gov>, E. Robert Tisdale
<E.**************@jpl.nasa.gov> writes

Ed Avis wrote:

Displaying a graphic doesn't really 'modify' that graphic,
so I agree, it should return void.

It modifies the display.

And if that is done by a system call? What should I then return?

There are only three possibilities.

1) The system call can never fail, so there is no point in returning
anything.

2) The system call can fail, and no recovery is possible in the function
making the call. In this case the error needs to be reported.
That may be done by returning an error code or (if an error should
never be ignored) throwing an exception.

3) If the system call can fail, but recovery is possible by the function
making the call, then the appropriate action needs to be determined
by what the caller needs to be told about. [This is a mix of 1 and
2].

In the end it comes down to what the caller needs to know about and what
it can reasonably be expected to cope with.


[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

[kanze]

br******@cix.co.uk (Dave Harris) wrote in message
news:<me************************@brangdon.madasafi sh.com>...

E.**************@jpl.nasa.gov (E. Robert Tisdale) wrote (abridged):

Procedural programs are difficult to analyze and practically
impossible to prove correct. You should avoid procedure calls (and
assignment statements) whenever possible.

Right. And you should make sure no-one confuses a procedure with a
function. One way to achieve that is to make sure functions never have
side effects and procedures never return results.

So what do you do with rand()? (I like the rule, but there are
exceptions.)

--
James Kanze GABI Software
mailto:ka***@gabi-soft.fr
Conseils en informatique orientée objet/
http://www.gabi-soft.fr
Beratung in objektorientierter
Datenverarbeitung
11 rue de Rambouillet, 78460 Chevreuse, France, Tél. : [phone]
16

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

[Dave Harris]

ka***@gabi-soft.fr () wrote (abridged):

Right. And you should make sure no-one confuses a procedure with
a function. One way to achieve that is to make sure functions
never have side effects and procedures never return results.
So what do you do with rand()?

Rand ought to be an object anyway:

class {
int state; // or whatever
public:
void srand( int seed );
void advance();
int current() const;
// int next() const { advance(); return current(); }
};

(I like the rule, but there are exceptions.)

I agree there are exceptions. Another possible exception is having a
procedure return information about whether it succeeded.

These can be eliminated, of course, and some people say they always
should be, but I don't take that extreme view. The circumlocutions
sometimes have drawbacks which are worse than the problem they
solve.

-- Dave Harris, Nottingham, UK

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

[Francis Glassborow]

In message <3F**************@jpl.nasa.gov>, E. Robert Tisdale
<E.**************@jpl.nasa.gov> writes

Francis Glassborow wrote:

And you were responding to Mr. Tisdale's rewriting of what I had written.

Your function

void foo(void){
std::cout << std::clock();
}

modifies a global variable -- cout.

So? being a global it is available for anyone to check. Indeed if they
are really paranoid they can switch on exception behaviour for streams.
--
ACCU Spring Conference 2003 April 2-5
The Conference you should not have missed
ACCU Spring Conference 2004 Late April
Francis Glassborow ACCU
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

[Francis Glassborow]

In message <l1************@budvar.future-i.net>, Ed Avis
<[e-mail]> writes

I am just thinking of a relatively high-level interface such as

class Screen {
XXX display(Graphic);
};

XXX could be void or it could be Screen&, returning *this. The latter
style would let you chain display() calls, if you are so inclined.

And if you read my original guideline it said something that included
that possibility. Tisdale may have taken us down this path but he was
disagreeing with a guideline that most seem to approve of. I wasn't the
OP but my guidelines were a response to his post. Tisdale claimed that
the guideline was a bad one. I have seen nothing to change my mind.

--
ACCU Spring Conference 2003 April 2-5
The Conference you should not have missed
ACCU Spring Conference 2004 Late April
Francis Glassborow ACCU
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

[E. Robert Tisdale]

James Kanze:

Dave Harris) wrote:

And you should make sure no-one confuses a procedure with a function.
One way to achieve that is to make sure that functions
never have side effects and procedures never return results.

So what do you do with rand()?
(I like the rule, but there are exceptions.)

The rand() function was a *bad* idea for a number of different reasons.
It should be replace with a Uniform Random Number Generator

class URNG {
private:
// state representation
public:
// operators
int operator()(void);
// constructors
URNG(unsigned int seed);
};

int main(int argc, char* argv[]) {
URNG rand(0);
// use rand()
return 0;
}
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

[llewelly]

"E. Robert Tisdale" <E.**************@jpl.nasa.gov> writes:

James Kanze:

Dave Harris) wrote:

And you should make sure no-one confuses a procedure with a function.
One way to achieve that is to make sure that functions
never have side effects and procedures never return results.
So what do you do with rand()?
(I like the rule, but there are exceptions.)

The rand() function was a *bad* idea for a number of different

reasons. It should be replace with a Uniform Random Number Generator

class URNG {
Yes, however, let's please not give it a name which looks like a cross
between a macro and punch to the gut. Anyway, boost has some
rather nice random number generators.
private:
// state representation
public:
// operators
int operator()(void);
// constructors
URNG(unsigned int seed);
};

int main(int argc, char* argv[]) {
URNG rand(0);
// use rand()
return 0;
}

[snip]

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]