Code critique

**Petec** · Jul 22 '05, 09:03 AM

Re: Code critique

Martin Eisenberg wrote:[color=blue]
> Hi,
>
> I've written a program that I'd like to hear some opinions on
> regarding my C++ usage. As a program it's smallish, but on Usenet 300
> lines seem a bit much. Do you think it's appropriate to post the
> code? Alternatively, maybe someone would be willing to put it on the
> web for some days? I'm reluctant to sign up with a freespace provider
> just for this one occasion; besides, most disallow use of their
> space as pure file storage.
>
> Thanks,
> Martin[/color]

I'd be happy to critique your code, and I think as long as you put (long) in
the subject it'd be fine. :)

- Pete

**Petec** · Jul 22 '05, 09:03 AM

Re: Code critique

Martin Eisenberg wrote:[color=blue]
> Hi,
>
> I've written a program that I'd like to hear some opinions on
> regarding my C++ usage. As a program it's smallish, but on Usenet 300
> lines seem a bit much. Do you think it's appropriate to post the
> code? Alternatively, maybe someone would be willing to put it on the
> web for some days? I'm reluctant to sign up with a freespace provider
> just for this one occasion; besides, most disallow use of their
> space as pure file storage.
>
> Thanks,
> Martin[/color]

I'd be happy to critique your code, and I think as long as you put (long) in
the subject it'd be fine. :)

- Pete

**David Harmon** · Jul 22 '05, 09:03 AM

Re: Code critique

On 6 Apr 2004 22:31:34 GMT in comp.lang.c++, Martin Eisenberg
<martin.eisenbe rgNOS@PAMudo.ed u> wrote,[color=blue]
>I've written a program that I'd like to hear some opinions on
>regarding my C++ usage. As a program it's smallish, but on Usenet 300
>lines seem a bit much. Do you think it's appropriate to post the
>code?[/color]

I think you should ask the specific questions that you wish to ask, and
post as much code as is needed for the context for the questions. The
more specific the questions, the better answers you will get. The more
relevant the code is to the specific questions, the better answers you
will get.

**David Harmon** · Jul 22 '05, 09:03 AM

Re: Code critique

On 6 Apr 2004 22:31:34 GMT in comp.lang.c++, Martin Eisenberg
<martin.eisenbe rgNOS@PAMudo.ed u> wrote,[color=blue]
>I've written a program that I'd like to hear some opinions on
>regarding my C++ usage. As a program it's smallish, but on Usenet 300
>lines seem a bit much. Do you think it's appropriate to post the
>code?[/color]

I think you should ask the specific questions that you wish to ask, and
post as much code as is needed for the context for the questions. The
more specific the questions, the better answers you will get. The more
relevant the code is to the specific questions, the better answers you
will get.

**Martin Eisenberg** · Jul 22 '05, 09:03 AM

Re: Code critique

David Harmon wrote:
[color=blue]
> I think you should ask the specific questions that you wish to
> ask,[/color]

Is my code in good style from the Standard C++ viewpoint? It doesn't
get any more specific than that.
[color=blue]
> and post as much code as is needed for the context for the
> questions.[/color]

Which would be the whole program.

Martin

**Martin Eisenberg** · Jul 22 '05, 09:03 AM

Re: Code critique

David Harmon wrote:
[color=blue]
> I think you should ask the specific questions that you wish to
> ask,[/color]

Is my code in good style from the Standard C++ viewpoint? It doesn't
get any more specific than that.
[color=blue]
> and post as much code as is needed for the context for the
> questions.[/color]

Which would be the whole program.

Martin

**Alf P. Steinbach** · Jul 22 '05, 09:03 AM

Re: Code critique

* Martin Eisenberg <martin.eisenbe rgNOS@PAMudo.ed u> schriebt:[color=blue]
> David Harmon wrote:
>[color=green]
> > I think you should ask the specific questions that you wish to
> > ask,[/color]
>
> Is my code in good style from the Standard C++ viewpoint? It doesn't
> get any more specific than that.
>[color=green]
> > and post as much code as is needed for the context for the
> > questions.[/color]
>
> Which would be the whole program.[/color]

Just post, put "(long)" in the subject line.

--
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?

**Alf P. Steinbach** · Jul 22 '05, 09:03 AM

Re: Code critique

* Martin Eisenberg <martin.eisenbe rgNOS@PAMudo.ed u> schriebt:[color=blue]
> David Harmon wrote:
>[color=green]
> > I think you should ask the specific questions that you wish to
> > ask,[/color]
>
> Is my code in good style from the Standard C++ viewpoint? It doesn't
> get any more specific than that.
>[color=green]
> > and post as much code as is needed for the context for the
> > questions.[/color]
>
> Which would be the whole program.[/color]

Just post, put "(long)" in the subject line.

--
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?

**Martin Eisenberg** · Jul 22 '05, 09:03 AM

Re: Code critique

OK, thanks for the encouragements. Note that I'm well aware that
I won't solve the Collatz conjecture that way. It is the canvas,
C++ is the picture. Also, I know the standard doesn't speak to
those MSVC artifacts, but if you think they could be better
handled I'm very interested in hearing about that as well.
There are three files; word wrap is off.

Martin

// main.cpp
/* This program searches for a seed value
yielding a Collatz sequence that is long
in relation to the seed's magnitude.
*/

#ifdef _MSC_VER
// "identifier was truncated to '255' characters in the debug information"
#pragma warning(disable : 4786)
#endif

#include <iostream>
#include <iomanip>
#include <fstream>
#include <string>
#include "collatztre e.h"

using namespace std;

const string KSeqFile = "collatz.tx t";
const unsigned KCheckInterval = 10000000;

//-----------------------------------------------

void awaitEnter()
{
char c[2];
cout << "Program finished. Hit <Enter> to close.";
cin.getline(c, 2);
}

bool prompt(const string& message)
{
char c;
cout << message << " (y/n <Enter>) ";
cin >> c;
#ifdef WIN32
cin.ignore(); // Swallow Windows newline's second character.
#endif
cout << '\n';
return c == 'y';
}

bool checkUserBreak( )
{ return !prompt("Contin ue searching?"); }

void writeBest(const CollatzTree::Be st& best,
ostream& os, unsigned nodeCount = 0)
{
os << "The best seed found ";
if(nodeCount > 0) os << "among the first " << nodeCount << " nodes ";
os << "is n = " << best.pNode->value
<< "\nwith number of iterations it(n) = " << best.depth
<< "\nand cost c(n) = it(n) / log2(n) = " << best.cost << ".\n";
}

void writeBest(const CollatzTree::Be st& best, unsigned nodeCount)
{ writeBest(best, cout, nodeCount); }

void writeSequence(c onst CollatzTree::Be st& best)
{
if(prompt("Writ e the longest sequence found to " + KSeqFile + "?")) {
ofstream ofs(KSeqFile.c_ str());
writeBest(best, ofs);
ofs << '\n';
CollatzTree::PN ode pn = best.pNode;
ofs << pn->value;
while(!pn->isRoot()) {
pn = pn->parent;
ofs << ", " << pn->value;
}
ofs << '\n';
}
}

//-----------------------------------------------

int main()
{
CollatzTree ct(writeBest, checkUserBreak, KCheckInterval) ;
cout << "Building Collatz tree...\n\n" << fixed << setprecision(3) ;
do ct.addLevel(); while(!ct.isFin ished());
writeSequence(c t.getBest());
awaitEnter();
return 0;
}

// end of file ----------------------------------------------------------------

// collatztree.h
#ifndef COLLATZTREE_H
#define COLLATZTREE_H

#include <vector>
#include <list>
#include <utility>
#include <memory>

/* The class CollatzTree creates, level for level, the
tree of terminating Collatz sequences that do not
exceed the range of type unsigned. It also tracks
the seed value n with largest quotient of the number
of iterations from n and log2(n), and periodically
outputs the most expensive seed found so far.
Smaller values are favored in case of equal cost.
The user can interrupt the class' operation.
The possibility of memory exhaustion is ignored.
*/

class CollatzTree {
public:
struct Node;
struct Best;
typedef void (*WriteBest)(co nst Best&, unsigned nodeCount);
typedef bool (*CheckUserBrea k)();
typedef const Node* PNode;

struct Node {
Node(unsigned value, PNode parent);
bool isRoot() const;

unsigned value;
PNode parent;
};

struct Best {
PNode pNode;
unsigned depth;
float cost;
};

//--------------

CollatzTree(Wri teBest writeBest,
CheckUserBreak checkUserBreak, unsigned checkInterval);
~CollatzTree();
void addLevel();
const Best& getBest() const;
bool isFinished() const;

private:
typedef std::vector<Nod e> Level;
typedef std::vector<Lev el*> Levels;
typedef std::pair<unsig ned, unsigned> Children;

PNode addInitial(unsi gned value, PNode parent);
bool insert(unsigned value, PNode parent);
Children getChildren(uns igned value);
size_t getNewLevelSize ();

const WriteBest writeBest_;
const CheckUserBreak checkUserBreak_ ;
const unsigned checkInterval_;
Levels levels_;
Best best_;
unsigned nodeCount_;
bool isFinished_;
};

#endif // COLLATZTREE_H
// end of file ----------------------------------------------------------------

// collatztree.cpp
#include "collatztre e.h"

#if defined(_MSC_VE R) && _MSC_VER == 1200 // MSVC 6
namespace std {
#include <math.h>
}
#else
#include <cmath>
#endif

#include <limits>

// CollatzTree ----------------------------------------------------------------
// public

CollatzTree::No de::Node(unsign ed value, CollatzTree::PN ode parent)
: value(value), parent(parent)
{}

bool CollatzTree::No de::isRoot() const
{ return parent == 0; }

//-----------------------------------------------

/* The constructor creates the entries up to 8 so that
getChildren() need not handle 1 as a spurious child of 4.
*/

CollatzTree::Co llatzTree(Colla tzTree::WriteBe st writeBest,
CollatzTree::Ch eckUserBreak checkUserBreak, unsigned checkInterval)
: writeBest_(writ eBest), checkUserBreak_ (checkUserBreak ),
checkInterval_( checkInterval), nodeCount_(4), isFinished_(fal se)
{
PNode pn = addInitial(1, 0);
pn = addInitial(2, pn);
best_.pNode = pn;
best_.depth = 1;
best_.cost = 1;
addInitial(8, addInitial(4, pn));
}

CollatzTree::~C ollatzTree()
{
Levels::iterato r pl = levels_.begin() ;
for(; pl != levels_.end(); ++pl) delete *pl;
}

/* The method addLevel() finds the numbers that a single
Collatz iteration takes to those in the previous top level.
It controls the new level's capacity to avoid memory waste.
*/

void CollatzTree::ad dLevel()
{
if(isFinished_) return;
const Level& oldTop = *levels_.back() ;
levels_.push_ba ck(new Level());
levels_.back()->reserve(getNew LevelSize());

Children children;
Level::const_it erator parent = oldTop.begin();
for(; parent != oldTop.end() && !isFinished_; ++parent) {
children = getChildren(par ent->value);
if(children.fir st != 0)
isFinished_ = insert(children .first, parent);
if(children.sec ond != 0 && !isFinished_)
isFinished_ = insert(children .second, parent);
}
}

const CollatzTree::Be st& CollatzTree::ge tBest() const
{ return best_; }

bool CollatzTree::is Finished() const
{ return isFinished_; }

// private --------------------------------------

CollatzTree::PN ode
CollatzTree::ad dInitial(unsign ed value, CollatzTree::PN ode parent)
{
Level* pl = new Level();
levels_.push_ba ck(pl);
pl->push_back(Node (value, parent));
return &pl->back();
}

bool CollatzTree::in sert(unsigned value, CollatzTree::PN ode parent)
{
static const float KLn2 = std::log(2.f);
static const unsigned KMaxNodeCount = std::numeric_li mits<unsigned>: :max();

levels_.back()->push_back(Node (value, parent));
unsigned depth = levels_.size() - 1;
float cost(depth / std::log(float( value)) * KLn2);
if(cost >= best_.cost && (cost > best_.cost || value < best_.pNode->value)) {
best_.pNode = &levels_.bac k()->back();
best_.depth = depth;
best_.cost = cost;
}
if(++nodeCount_ % checkInterval_ == 0) {
writeBest_(best _, nodeCount_);
if(checkUserBre ak_()) return true;
}
return nodeCount_ == KMaxNodeCount;
}

CollatzTree::Ch ildren CollatzTree::ge tChildren(unsig ned value)
{
static const unsigned
KHalfMaxValue = std::numeric_li mits<unsigned>: :max() / 2;

unsigned c1 = value * 2, c2 = (value - 1) / 3;
if(value > KHalfMaxValue) c1 = 0;
if(c2 % 2 == 0 || c2 * 3 + 1 != value) c2 = 0;
return Children(c1, c2);
}

/* Empirically determining the factors in getNewLevelSize () is easy,
but I have not proven that there are no outliers further up.
*/

size_t CollatzTree::ge tNewLevelSize()
{
static const float KFactors[] = {
0.f, 1.f, 1.f, 1.f, 1.f, 2.f, 1.f, 2.f, 1.f, 1.5f, 1.f,
1.34f, 1.25f, 1.4f, 1.29f, 1.34f, 1.21f, 1.25f, 1.23f, 1.32f
};
static const size_t NFactors(sizeof (KFactors) / sizeof(KFactors[0]));

float factor;
unsigned depth = levels_.size() - 1;
if(depth < NFactors) factor = KFactors[depth]; else factor = 1.28f;
return static_cast<siz e_t>(std::ceil( levels_[depth - 1]->size() * factor));
}

// end of file ----------------------------------------------------------------

**Martin Eisenberg** · Jul 22 '05, 09:03 AM

Re: Code critique

OK, thanks for the encouragements. Note that I'm well aware that
I won't solve the Collatz conjecture that way. It is the canvas,
C++ is the picture. Also, I know the standard doesn't speak to
those MSVC artifacts, but if you think they could be better
handled I'm very interested in hearing about that as well.
There are three files; word wrap is off.

Martin

// main.cpp
/* This program searches for a seed value
yielding a Collatz sequence that is long
in relation to the seed's magnitude.
*/

#ifdef _MSC_VER
// "identifier was truncated to '255' characters in the debug information"
#pragma warning(disable : 4786)
#endif

#include <iostream>
#include <iomanip>
#include <fstream>
#include <string>
#include "collatztre e.h"

using namespace std;

const string KSeqFile = "collatz.tx t";
const unsigned KCheckInterval = 10000000;

//-----------------------------------------------

void awaitEnter()
{
char c[2];
cout << "Program finished. Hit <Enter> to close.";
cin.getline(c, 2);
}

bool prompt(const string& message)
{
char c;
cout << message << " (y/n <Enter>) ";
cin >> c;
#ifdef WIN32
cin.ignore(); // Swallow Windows newline's second character.
#endif
cout << '\n';
return c == 'y';
}

bool checkUserBreak( )
{ return !prompt("Contin ue searching?"); }

void writeBest(const CollatzTree::Be st& best,
ostream& os, unsigned nodeCount = 0)
{
os << "The best seed found ";
if(nodeCount > 0) os << "among the first " << nodeCount << " nodes ";
os << "is n = " << best.pNode->value
<< "\nwith number of iterations it(n) = " << best.depth
<< "\nand cost c(n) = it(n) / log2(n) = " << best.cost << ".\n";
}

void writeBest(const CollatzTree::Be st& best, unsigned nodeCount)
{ writeBest(best, cout, nodeCount); }

void writeSequence(c onst CollatzTree::Be st& best)
{
if(prompt("Writ e the longest sequence found to " + KSeqFile + "?")) {
ofstream ofs(KSeqFile.c_ str());
writeBest(best, ofs);
ofs << '\n';
CollatzTree::PN ode pn = best.pNode;
ofs << pn->value;
while(!pn->isRoot()) {
pn = pn->parent;
ofs << ", " << pn->value;
}
ofs << '\n';
}
}

//-----------------------------------------------

int main()
{
CollatzTree ct(writeBest, checkUserBreak, KCheckInterval) ;
cout << "Building Collatz tree...\n\n" << fixed << setprecision(3) ;
do ct.addLevel(); while(!ct.isFin ished());
writeSequence(c t.getBest());
awaitEnter();
return 0;
}

// end of file ----------------------------------------------------------------

// collatztree.h
#ifndef COLLATZTREE_H
#define COLLATZTREE_H

#include <vector>
#include <list>
#include <utility>
#include <memory>

/* The class CollatzTree creates, level for level, the
tree of terminating Collatz sequences that do not
exceed the range of type unsigned. It also tracks
the seed value n with largest quotient of the number
of iterations from n and log2(n), and periodically
outputs the most expensive seed found so far.
Smaller values are favored in case of equal cost.
The user can interrupt the class' operation.
The possibility of memory exhaustion is ignored.
*/

class CollatzTree {
public:
struct Node;
struct Best;
typedef void (*WriteBest)(co nst Best&, unsigned nodeCount);
typedef bool (*CheckUserBrea k)();
typedef const Node* PNode;

struct Node {
Node(unsigned value, PNode parent);
bool isRoot() const;

unsigned value;
PNode parent;
};

struct Best {
PNode pNode;
unsigned depth;
float cost;
};

//--------------

CollatzTree(Wri teBest writeBest,
CheckUserBreak checkUserBreak, unsigned checkInterval);
~CollatzTree();
void addLevel();
const Best& getBest() const;
bool isFinished() const;

private:
typedef std::vector<Nod e> Level;
typedef std::vector<Lev el*> Levels;
typedef std::pair<unsig ned, unsigned> Children;

PNode addInitial(unsi gned value, PNode parent);
bool insert(unsigned value, PNode parent);
Children getChildren(uns igned value);
size_t getNewLevelSize ();

const WriteBest writeBest_;
const CheckUserBreak checkUserBreak_ ;
const unsigned checkInterval_;
Levels levels_;
Best best_;
unsigned nodeCount_;
bool isFinished_;
};

#endif // COLLATZTREE_H
// end of file ----------------------------------------------------------------

// collatztree.cpp
#include "collatztre e.h"

#if defined(_MSC_VE R) && _MSC_VER == 1200 // MSVC 6
namespace std {
#include <math.h>
}
#else
#include <cmath>
#endif

#include <limits>

// CollatzTree ----------------------------------------------------------------
// public

CollatzTree::No de::Node(unsign ed value, CollatzTree::PN ode parent)
: value(value), parent(parent)
{}

bool CollatzTree::No de::isRoot() const
{ return parent == 0; }

//-----------------------------------------------

/* The constructor creates the entries up to 8 so that
getChildren() need not handle 1 as a spurious child of 4.
*/

CollatzTree::Co llatzTree(Colla tzTree::WriteBe st writeBest,
CollatzTree::Ch eckUserBreak checkUserBreak, unsigned checkInterval)
: writeBest_(writ eBest), checkUserBreak_ (checkUserBreak ),
checkInterval_( checkInterval), nodeCount_(4), isFinished_(fal se)
{
PNode pn = addInitial(1, 0);
pn = addInitial(2, pn);
best_.pNode = pn;
best_.depth = 1;
best_.cost = 1;
addInitial(8, addInitial(4, pn));
}

CollatzTree::~C ollatzTree()
{
Levels::iterato r pl = levels_.begin() ;
for(; pl != levels_.end(); ++pl) delete *pl;
}

/* The method addLevel() finds the numbers that a single
Collatz iteration takes to those in the previous top level.
It controls the new level's capacity to avoid memory waste.
*/

void CollatzTree::ad dLevel()
{
if(isFinished_) return;
const Level& oldTop = *levels_.back() ;
levels_.push_ba ck(new Level());
levels_.back()->reserve(getNew LevelSize());

Children children;
Level::const_it erator parent = oldTop.begin();
for(; parent != oldTop.end() && !isFinished_; ++parent) {
children = getChildren(par ent->value);
if(children.fir st != 0)
isFinished_ = insert(children .first, parent);
if(children.sec ond != 0 && !isFinished_)
isFinished_ = insert(children .second, parent);
}
}

const CollatzTree::Be st& CollatzTree::ge tBest() const
{ return best_; }

bool CollatzTree::is Finished() const
{ return isFinished_; }

// private --------------------------------------

CollatzTree::PN ode
CollatzTree::ad dInitial(unsign ed value, CollatzTree::PN ode parent)
{
Level* pl = new Level();
levels_.push_ba ck(pl);
pl->push_back(Node (value, parent));
return &pl->back();
}

bool CollatzTree::in sert(unsigned value, CollatzTree::PN ode parent)
{
static const float KLn2 = std::log(2.f);
static const unsigned KMaxNodeCount = std::numeric_li mits<unsigned>: :max();

levels_.back()->push_back(Node (value, parent));
unsigned depth = levels_.size() - 1;
float cost(depth / std::log(float( value)) * KLn2);
if(cost >= best_.cost && (cost > best_.cost || value < best_.pNode->value)) {
best_.pNode = &levels_.bac k()->back();
best_.depth = depth;
best_.cost = cost;
}
if(++nodeCount_ % checkInterval_ == 0) {
writeBest_(best _, nodeCount_);
if(checkUserBre ak_()) return true;
}
return nodeCount_ == KMaxNodeCount;
}

CollatzTree::Ch ildren CollatzTree::ge tChildren(unsig ned value)
{
static const unsigned
KHalfMaxValue = std::numeric_li mits<unsigned>: :max() / 2;

unsigned c1 = value * 2, c2 = (value - 1) / 3;
if(value > KHalfMaxValue) c1 = 0;
if(c2 % 2 == 0 || c2 * 3 + 1 != value) c2 = 0;
return Children(c1, c2);
}

/* Empirically determining the factors in getNewLevelSize () is easy,
but I have not proven that there are no outliers further up.
*/

size_t CollatzTree::ge tNewLevelSize()
{
static const float KFactors[] = {
0.f, 1.f, 1.f, 1.f, 1.f, 2.f, 1.f, 2.f, 1.f, 1.5f, 1.f,
1.34f, 1.25f, 1.4f, 1.29f, 1.34f, 1.21f, 1.25f, 1.23f, 1.32f
};
static const size_t NFactors(sizeof (KFactors) / sizeof(KFactors[0]));

float factor;
unsigned depth = levels_.size() - 1;
if(depth < NFactors) factor = KFactors[depth]; else factor = 1.28f;
return static_cast<siz e_t>(std::ceil( levels_[depth - 1]->size() * factor));
}

// end of file ----------------------------------------------------------------

**Martin Eisenberg** · Jul 22 '05, 09:03 AM

Re: Code critique (above is long)

<sigh> ...and sorry for forgetting the "(long)" in the subject.

Martin

**Martin Eisenberg** · Jul 22 '05, 09:03 AM

Re: Code critique (above is long)

<sigh> ...and sorry for forgetting the "(long)" in the subject.

Martin

**Kevin Goodsell** · Jul 22 '05, 09:03 AM

Re: Code critique

Martin Eisenberg wrote:
[color=blue]
> OK, thanks for the encouragements. Note that I'm well aware that
> I won't solve the Collatz conjecture that way. It is the canvas,
> C++ is the picture. Also, I know the standard doesn't speak to
> those MSVC artifacts, but if you think they could be better
> handled I'm very interested in hearing about that as well.
> There are three files; word wrap is off.[/color]

This is just a few quick comments, no time for a complete run-down right
now.
[color=blue]
>
>
> Martin
>
> // main.cpp
> /* This program searches for a seed value
> yielding a Collatz sequence that is long
> in relation to the seed's magnitude.
> */
>
> #ifdef _MSC_VER
> // "identifier was truncated to '255' characters in the debug information"
> #pragma warning(disable : 4786)
> #endif[/color]

Sucks, don't it? Not much else you can do about this, though.
[color=blue]
>
> #include <iostream>
> #include <iomanip>
> #include <fstream>
> #include <string>
> #include "collatztre e.h"
>
> using namespace std;[/color]

This should be avoided, but is not terrible, particularly for small
programs. Generally, import only what you need where you need it, so you
could use

using std::string;
using std::vector;

and you could even put these inside the functions that use those items.
[color=blue]
>
> const string KSeqFile = "collatz.tx t";
> const unsigned KCheckInterval = 10000000;[/color]

This value is much too large to portably put in an unsigned int. The
portable maximum for unsigned int is 65535. Better make it unsigned long.
[color=blue]
>
> //-----------------------------------------------
>
> void awaitEnter()
> {
> char c[2];
> cout << "Program finished. Hit <Enter> to close.";
> cin.getline(c, 2);
> }
>
> bool prompt(const string& message)
> {
> char c;
> cout << message << " (y/n <Enter>) ";
> cin >> c;
> #ifdef WIN32
> cin.ignore(); // Swallow Windows newline's second character.
> #endif[/color]

Eh? I don't think this is right. This should be the same, Windows or
not. Besides that, you can't control how many characters are actually
entered, so you should probably extract the entire line, and check the
first character.

That's about all I can do right now.

-Kevin
--
My email address is valid, but changes periodically.
To contact me please use the address from a recent posting.

**Kevin Goodsell** · Jul 22 '05, 09:03 AM

Re: Code critique

Martin Eisenberg wrote:
[color=blue]
> OK, thanks for the encouragements. Note that I'm well aware that
> I won't solve the Collatz conjecture that way. It is the canvas,
> C++ is the picture. Also, I know the standard doesn't speak to
> those MSVC artifacts, but if you think they could be better
> handled I'm very interested in hearing about that as well.
> There are three files; word wrap is off.[/color]

This is just a few quick comments, no time for a complete run-down right
now.
[color=blue]
>
>
> Martin
>
> // main.cpp
> /* This program searches for a seed value
> yielding a Collatz sequence that is long
> in relation to the seed's magnitude.
> */
>
> #ifdef _MSC_VER
> // "identifier was truncated to '255' characters in the debug information"
> #pragma warning(disable : 4786)
> #endif[/color]

Sucks, don't it? Not much else you can do about this, though.
[color=blue]
>
> #include <iostream>
> #include <iomanip>
> #include <fstream>
> #include <string>
> #include "collatztre e.h"
>
> using namespace std;[/color]

This should be avoided, but is not terrible, particularly for small
programs. Generally, import only what you need where you need it, so you
could use

using std::string;
using std::vector;

and you could even put these inside the functions that use those items.
[color=blue]
>
> const string KSeqFile = "collatz.tx t";
> const unsigned KCheckInterval = 10000000;[/color]

This value is much too large to portably put in an unsigned int. The
portable maximum for unsigned int is 65535. Better make it unsigned long.
[color=blue]
>
> //-----------------------------------------------
>
> void awaitEnter()
> {
> char c[2];
> cout << "Program finished. Hit <Enter> to close.";
> cin.getline(c, 2);
> }
>
> bool prompt(const string& message)
> {
> char c;
> cout << message << " (y/n <Enter>) ";
> cin >> c;
> #ifdef WIN32
> cin.ignore(); // Swallow Windows newline's second character.
> #endif[/color]

Eh? I don't think this is right. This should be the same, Windows or
not. Besides that, you can't control how many characters are actually
entered, so you should probably extract the entire line, and check the
first character.

That's about all I can do right now.

-Kevin
--
My email address is valid, but changes periodically.
To contact me please use the address from a recent posting.

Code critique

Code critique

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment