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
--
Quidquid latine dictum sit, altum viditur.
Jul 22 '05
54 2841
Kevin Goodsell wrote: Martin Eisenberg wrote:
Conclusion using Modus Praematurus: - I need to special-case for Windows.
You did notice that was tongue-in-cheek, yes?
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
"Martin Eisenberg" <ma*****************@PAMudo.edu> wrote in message
news:10***************@ostenberg.wh.uni-dortmund.de...
Hi Martin, cin.ignore( std::numeric_limits<int>::max(), '\n' );
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) typedef const Node* PNode; The first two typedefs are very reasonable, but I would recommend dropping PNode: It is usually best to be explicit about pointers and their const-qualifications -- use Node* or Node const* instead.
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) - Use a container such as std::list or std::deque to store any objects you allocate -- i.e. replace: Level* p = new Level(); with levelPool.push_back( Level() ); Level* p = &levelPool.back(); after adding a data member such as: std::deque<Level> levelPool; At destruction time, the 'levelPool' container will safely destroy and deallocate all Level instances.
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. Unfortunately, this is not enough to avoid all memory leaks. For example, if your constructor (which adds items) fails with an exception (e.g. during a memory allocation), objects will be leaked.
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. void CollatzTree::addLevel() { if(isFinished_) return; const Level& oldTop = *levels_.back(); levels_.push_back(new Level()); levels_.back()->reserve(getNewLevelSize());
Here too, if the push_back() call fails, you will be left with a memory leak.
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
-- http://ivan.vecerina.com/contact/?subject=NG_POST <- e-mail contact form
"Martin Eisenberg" <ma*****************@PAMudo.edu> wrote in message
news:10***************@ostenberg.wh.uni-dortmund.de...
Hi Martin, cin.ignore( std::numeric_limits<int>::max(), '\n' );
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) typedef const Node* PNode; The first two typedefs are very reasonable, but I would recommend dropping PNode: It is usually best to be explicit about pointers and their const-qualifications -- use Node* or Node const* instead.
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) - Use a container such as std::list or std::deque to store any objects you allocate -- i.e. replace: Level* p = new Level(); with levelPool.push_back( Level() ); Level* p = &levelPool.back(); after adding a data member such as: std::deque<Level> levelPool; At destruction time, the 'levelPool' container will safely destroy and deallocate all Level instances.
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. Unfortunately, this is not enough to avoid all memory leaks. For example, if your constructor (which adds items) fails with an exception (e.g. during a memory allocation), objects will be leaked.
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. void CollatzTree::addLevel() { if(isFinished_) return; const Level& oldTop = *levels_.back(); levels_.push_back(new Level()); levels_.back()->reserve(getNewLevelSize());
Here too, if the push_back() call fails, you will be left with a memory leak.
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
-- http://ivan.vecerina.com/contact/?subject=NG_POST <- e-mail contact form
Ivan Vecerina wrote: Hi Martin,
Hey!
<abridged>
"Martin Eisenberg" <ma*****************@PAMudo.edu> wrote in message news:10***************@ostenberg.wh.uni-dortmund.de...
>> typedef const Node* PNode;
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)
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.
> after adding a data member such as: > std::deque<Level> levelPool;
/-: Sorry, somehow these lines did not make it into consciousness.
There's no question here.
Or maybe build a local structure and assign that to the member if all is well in the end?
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
--
It is not the strongest of the species
that survive, nor the most intelligent,
but the one most responsive to change.
--Charles Darwin
Ivan Vecerina wrote: Hi Martin,
Hey!
<abridged>
"Martin Eisenberg" <ma*****************@PAMudo.edu> wrote in message news:10***************@ostenberg.wh.uni-dortmund.de...
>> typedef const Node* PNode;
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)
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.
> after adding a data member such as: > std::deque<Level> levelPool;
/-: Sorry, somehow these lines did not make it into consciousness.
There's no question here.
Or maybe build a local structure and assign that to the member if all is well in the end?
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
--
It is not the strongest of the species
that survive, nor the most intelligent,
but the one most responsive to change.
--Charles Darwin This thread has been closed and replies have been disabled. Please start a new discussion. Similar topics
by: hokiegal99 |
last post by:
Is there a forum where one could post a Python script and have it
critiqued by others?
Something like: Y would be more efficent if you did it this way, or
doing it that way could cause problems...
|
by: Eric |
last post by:
There is a VB.NET critique on the following page:
http://www.vb7-critique.741.com/
for those who are interested. Feel free to take a look and share your
thoughts.
Cheers, Eric.
Ps: for those...
|
by: Martin Eisenberg |
last post by:
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...
|
by: TC |
last post by:
Are there any good sites or forums for a web critique? I went to
alt.html.critique and it's pretty dead.
|
by: bowsayge |
last post by:
Inspired by fb, Bowsayge decided to write a decimal integer
to binary string converter. Perhaps some of the experienced
C programmers here can critique it. It allocates probably
way too much...
|
by: G Patel |
last post by:
I wrote the following program to remove C89 type comments from stdin
and send it to stdout (as per exercise in K&R2) and it works but I was
hoping more experienced programmer would critique the...
|
by: christopher diggins |
last post by:
I have posted a C# critique at
http://www.heron-language.com/c-sharp-critique.html. To summarize I bring up
the following issues :
- unsafe code
- attributes
- garbage collection
-...
|
by: winston |
last post by:
I wrote a Python program (103 lines, below) to download developer data
from SourceForge for research about social networks.
Please critique the code and let me know how to improve it.
An...
|
by: matt |
last post by:
this is my first program in this language ever (besides 'hello
world'), can i get a code critique, please? it's purpose is to read
through an input file character by character and tally the...
|
by: DolphinDB |
last post by:
Tired of spending countless mintues downsampling your data? Look no further!
In this article, you’ll learn how to efficiently downsample 6.48 billion high-frequency records to 61 million...
|
by: isladogs |
last post by:
The next Access Europe meeting will be on Wednesday 6 Mar 2024 starting at 18:00 UK time (6PM UTC) and finishing at about 19:15 (7.15PM).
In this month's session, we are pleased to welcome back...
|
by: jfyes |
last post by:
As a hardware engineer, after seeing that CEIWEI recently released a new tool for Modbus RTU Over TCP/UDP filtering and monitoring, I actively went to its official website to take a look. It turned...
|
by: ArrayDB |
last post by:
The error message I've encountered is; ERROR:root:Error generating model response: exception: access violation writing 0x0000000000005140, which seems to be indicative of an access violation...
|
by: CloudSolutions |
last post by:
Introduction:
For many beginners and individual users, requiring a credit card and email registration may pose a barrier when starting to use cloud servers. However, some cloud server providers now...
|
by: Defcon1945 |
last post by:
I'm trying to learn Python using Pycharm but import shutil doesn't work
|
by: Shællîpôpï 09 |
last post by:
If u are using a keypad phone, how do u turn on JavaScript, to access features like WhatsApp, Facebook, Instagram....
|
by: Faith0G |
last post by:
I am starting a new it consulting business and it's been a while since I setup a new website. Is wordpress still the best web based software for hosting a 5 page website? The webpages will be...
|
by: isladogs |
last post by:
The next Access Europe User Group meeting will be on Wednesday 3 Apr 2024 starting at 18:00 UK time (6PM UTC+1) and finishing by 19:30 (7.30PM).
In this session, we are pleased to welcome former...
| |