Code critique

  • Thread starter Martin Eisenberg
  • Start date
B

Buster

K

Kevin Goodsell

Martin said:
Easy. Premises:
- newline has two characters on Windows and one on Unices.

This is not true, from a C++ perspective. End-of-line in a text stream
is always indicated with a single '\n' character. The implementation is
responsible for converting to and from the underlying representation
used by the system, so your code can (and should) ignore it.
- Every second call to the function excerpted at
the top does not wait for me to touch anything.

This sounds like either a bug in your implementation or possibly a
character left in the stream from some other part of the program.
Conclusion using Modus Praematurus:
- I need to special-case for Windows.

You shouldn't. I tried this simple test program using your code:

#include <ios>
#include <iostream>
#include <string>

using namespace std;

bool prompt(const string& message)
{
char c;
cout << message << " (y/n <Enter>) ";
cin >> c;
// cin.ignore();
cout << '\n';
return c == 'y';
}

int main()
{
cout << boolalpha;

while (true)
{
cout << "prompt() returned " << prompt("") << endl;
}
return 0;
}

In Visual C++ 6 (service pack 5, I think). It worked as expected.

-Kevin
 
M

Martin Eisenberg

Kevin said:
No, but my Latin is rusty.

The fun is, I don't know Latin at all! I explained how I came to the
(erroneous, I know that now) conclusion that the spurious newline
thing was a Windows problem, with a (probably ill-formed ;) play on
the inference rule Modus Ponens and like animals.


Martin
 
I

Ivan Vecerina

Hi Martin,
I've seen this before, but it didn't stick. To get really picky for a
moment, would size_t or streamsize or something be more fitting than
int?
(thanks to Buster for pointing out the related fix in the C++ standard)
What are some pitfalls that this abstraction would promote?

This might be seen as a matter of taste.
However, creating a new name (PNode) without actually
simplifying anything is more error prone than anything else.
I would have expected PNode to be a Node*, not a const Node*.
Other than the 'less typing' argument, is there any reason
not to just use "const Node*" instead of "PNode" ? (i.e. for clarity)
Did you intend levelPool to be of a class of my own creation which
takes ownership of inserted pointers? If not, what's the difference
to the other quoted option?
No: I intended it to be a standard container such as deque.
The aim is to keep using a container of pointers, which you
can efficiently sort, shuffle, move etc.
However, instead of allocating the pointed-to objects
on the heap, you allocate them within a separate container.
When this separate container is destroyed, you get
the equivalent of garbage collection for free.
Would it suffice to wrap the current ctor body in a try block and
then in the handler, put the above dtor code and rethrow? Or maybe
build a local structure and assign that to the member if all is well
in the end?

Adding a try..catch block usually isn't the simplest solution,
but it is an option. Alternatively, you could try using
a custom-made 'owning' container of pointers.
But I think the previously mentioned alternatives are better.
Level* pl = new Level();
try {
levels_.push_back(pl);
} catch(...) {
delete pl;
throw;
}

Is that reasonable?

It would seem easier to call levels_.reserve(...),
ensuring that no exception will be thrown.

In good (= reliable & maintainable) C++ code, uses of
try..catch blocks are be limited to locations where
errors are actually handled (or converted).
Rethrowing the same exception often hints at bad
design - IMHO.


Cheers,
Ivan
 
M

Martin Eisenberg

Ivan said:
Hi Martin,

Hey!

message news:[email protected]...

This might be seen as a matter of taste.
However, creating a new name (PNode) without actually
simplifying anything is more error prone than anything else.
I would have expected PNode to be a Node*, not a const Node*.
Other than the 'less typing' argument, is there any reason
not to just use "const Node*" instead of "PNode" ? (i.e. for
clarity)

I created the typedef because it is part of CollatzTree's public
interface. But now that you make me think about it, I see that it's
not really an abstraction at all -- what's hidden is so simple that
using PNode corretly requires full knowledge.

/-: Sorry, somehow these lines did not make it into consciousness.
There's no question here.

I know there is such an idiom. What is it called, so I can go ogle?
It would seem easier to call levels_.reserve(...),
ensuring that no exception will be thrown.

Yes, I now recall you already wrote that. Neat! ;)
In good (= reliable & maintainable) C++ code, uses of
try..catch blocks are be limited to locations where
errors are actually handled (or converted).
Rethrowing the same exception often hints at bad
design - IMHO.

That's interesting. I'll take a closer look at those points in my
code.

Thanks again, Ivan, for all the good advice!


Martin
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,744
Messages
2,569,483
Members
44,903
Latest member
orderPeak8CBDGummies

Latest Threads

Top