wwwtc6answer.htm
来自「C++builder学习资料C++builder」· HTM 代码 · 共 979 行 · 第 1/3 页
HTM
979 行
<HTML>
<HEAD>
<TITLE> Answer What's Wrong With This Code? Volume #6</TITLE>
<META NAME="Author" CONTENT="Harold Howe">
</HEAD>
<BODY>
<CENTER>
<TABLE BORDER=0 CELLPADDING=0 CELLSPACING=0 WIDTH="640">
<TR>
<TD>
<H2>
Answer What's Wrong With This Code? Volume #6
</H2>
<H4>
The STL<BR>
<I> by Chris Uzdavinis</I>
</H4>
<P>
For reference, each question is listed again, followed by its answer.
</P>
<pre>
<b>class</b> A
<b>{</b>
<b>public</b><b>:</b>
<b>void</b> do_something<b>(</b><b>)</b><b>;</b>
<b>bool</b> is_even<b>(</b><b>)</b> <b>const</b><b>;</b>
<b>}</b><b>;</b>
<b>typedef</b> std<b>:</b><b>:</b>vector<A<b>*</b><b>></b> V<b>;</b>
V v1<b>,</b> v2 <b>;</b>
</pre>
<P>
<B>Question #1:</B>
</P>
<pre>
<font color="navy">// assign contents of v2 to v1</font>
v1 <b>=</b> v2<b>;</b>
</pre>
<P>
What can potentially go wrong here? What considerations should
you make when doing this?
</P>
<P>
<B>Answer #1:</B>
</P>
<P>
<TT>v1 = v2</tT> has potential for memory leaks. Since <TT>v1</TT> gets overwritten,
any contents that it previously held are lost. If <TT>v1</TT> was the only
reference to those old values, they are leaked.
</P>
<P>
<TABLE WIDTH="75%">
<TR>
<TD VALIGN="top">
<IMG SRC="images/exclamation.gif" ALT="Tip" BORDER=0 HSPACE="0" ALIGN="top" width="32" height="48">
</TD>
<TD valign="top">
<b>Tip:</b>
<hr size = 1>
Remember to consider possible resource leaks when
removing elements from containers that hold pointers.
<hr size = 1>
</TD>
</TR>
</TABLE>
<P>
Correct code would (probably) first iterate through <TT>v1</TT> and call
<TT>delete</TT> on each element and then do the assignment, unless you can
guarantee that the objects have other references besides the
elements of <TT>v1</TT>.
</P>
<P>
<B>Question #2:</B>
</P>
<pre>
<font color="navy">// delete the element in position 5</font>
<b>void</b> f<b>(</b><b>)</b>
<b>{</b>
<b>if</b> <b>(</b>v1<b>.</b>size<b>(</b><b>)</b> <b>></b> <font color="blue">5</font><b>)</b>
<b>{</b>
<b>delete</b> v1<b>[</b><font color="blue">5</font><b>]</b><b>;</b>
<b>}</b>
<b>}</b>
</pre>
<P>
Why is calling <TT>f()</TT> most likely going to lead to a program crash?
</P>
<P>
<B>Answer #2:</B>
</P>
<P>
The error here is that after element 5 is deleted, the vector still
holds has the pointer to that deleted item. This is classic dangling
pointer bug. It is a good idea to remove <TT>v1[5]</TT> from the vector after you call
<TT>delete</TT>.
</P>
<P>
<P>
<TABLE WIDTH="75%">
<TR>
<TD VALIGN="top">
<IMG SRC="images/exclamation.gif" ALT="Note" BORDER=0 HSPACE="0" ALIGN="top" width="32" height="48">
</TD>
<TD valign="top">
<b>Tip:</b>
<hr size = 1>
Removing an element from a container will invalidate any
iterators currently pointing into the vector.
<hr size = 1>
</TD>
</TR>
</TABLE>
<P>
<B>Question #3:</B>
</P>
<pre>
<font color="navy">// copy v1 into v2</font>
<b>void</b> f<b>(</b><b>)</b>
<b>{</b>
std<b>:</b><b>:</b>copy<b>(</b>v1<b>.</b>begin<b>(</b><b>)</b><b>,</b> v1<b>.</b>end<b>(</b><b>)</b><b>,</b> v2<b>.</b>begin<b>(</b><b>)</b><b>)</b><b>;</b>
<b>}</b>
</pre>
<P>
How can this blow up?
</P>
<B>Answer #3:</B>
<P>
This is a correct call to copy. It copies the range of the first
two iterators into a corresponding range starting with the third
parameter, <TT>v2.begin()</TT>. Unfortunately, there is potential for
disastrous results. Notice, this is the entire body of a function,
which emphasizes the fact that this code is run in isolation.
</P>
<P>
What's the problem? There is no check to see that the destination range is
big enough to hold all the items. If <TT>v2</tt> is not large enough to hold
all of the items, the call to <TT>copy</TT> could cause a buffer overrun
and corrupt your program and hopefully crash it. Suppose vector <TT>v1</TT>
had a size of 10, but <TT>v2</TT> had a size of only 3. After the third
element is copied, the next element written to <TT>v2</TT> is out of bounds.
</P>
<P>
Remember that STL algorithms assume that their arguments are correct. They
don't perform any checking on their arguments. Therefore, it is up to the
author of <TT>f()</TT> to check that <TT>v2</TT> is at least as large as <TT>v1</TT>
before calling the copy algorithm.
</P>
<P>
Also, if <TT>v2</TT> is longer than <TT>v1</TT>, the extra elements at the end of
<TT>v2</TT> are not cleared. Thus, it's incorrect to assume <TT>v2</TT> to be an exact copy
of <TT>v1</TT> after returning from <TT>f()</TT>. All we can know is that for a <TT>v1</TT> of
size <TT>N</TT>, after returning from this call to <TT>copy()</TT>, <TT>v2</TT>'s first <TT>N</TT>
elements are copies of <TT>v1</TT>'s corresponding elements. Everything
after <TT>N</TT> in <TT>v2</TT> is unchanged. To ensure that <TT>v1</TT> and <TT>v2</TT> are identical
after the call to copy, resize <TT>v2</TT> to the size of <TT>v1</TT> first.
</P>
<P>
Here is a safer implementation:
</P>
<pre>
<font color="navy">// copy v1 into v2</font>
<b>void</b> f<b>(</b><b>)</b>
<b>{</b>
v2<b>.</b>resize<b>(</b>v1<b>.</b>size<b>(</b><b>)</b><b>)</b><b>;</b>
std<b>:</b><b>:</b>copy<b>(</b>v1<b>.</b>begin<b>(</b><b>)</b><b>,</b> v1<b>.</b>end<b>(</b><b>)</b><b>,</b> v2<b>.</b>begin<b>(</b><b>)</b><b>)</b><b>;</b>
<b>}</b>
</pre>
<P>
But that should sound off some alarm in your head. Why? Well,
you're reimplementing the <TT>operator=</TT> now, and probably in a less
efficient way than the provided <TT>operator=</TT>.
</P>
<P>
Therefore, I suggest replacing <TT>f()</TT> with:
</P>
<pre>
<b>void</b> f<b>(</b><b>)</b>
<b>{</b>
v2 <b>=</b> v1<b>;</b>
<b>}</b>
</pre>
<P>
All considerations for potential memory leaks from #1 above also
apply to this situation. If this will leak, we need to delete the items in <TT>v2</TT> first. For
pointers, using reference counts would be very helpful here.
</P>
<P>
<B>Question #4:</B>
</P>
<pre>
<font color="navy">// find an element and remove it from container.</font>
<font color="navy">// assume the caller keeps a reference to a so it</font>
<font color="navy">// isn't leaked.</font>
<b>void</b> f<b>(</b>A <b>*</b>a<b>)</b>
<b>{</b>
V<b>:</b><b>:</b>iterator i <b>=</b> std<b>:</b><b>:</b>find<b>(</b>v1<b>.</b>begin<b>(</b><b>)</b><b>,</b> v1<b>.</b>end<b>(</b><b>)</b><b>,</b> a<b>)</b><b>;</b>
<b>if</b> <b>(</b>i<b>)</b>
<b>{</b>
v1<b>.</b>erase<b>(</b>i<b>)</b><b>;</b>
<b>}</b>
<b>}</b>
</pre>
<P>
What is the subtle bug here that has serious consequences?
</P>
<P>
<B>Answer #4:</B>
</P>
<P>
This time, the problem is subtle. If you don't see it, take a second
look. The problem here is frequent enough and sinister enough to
spend a minute thinking about it. Hint: <TT>v1.erase(i)</TT> is *always* called!
</P>
<P>
What the programmer had intended to do was to search for the element
and remove it from the container if it was found. But the code contains a
subtle (and nasty!) bug.
</p>
<P>
The programmer correctly searches <TT>v1</TT> for a particular element a by
calling <TT>find()</TT>, and correctly gets back an iterator into the
sequence. So far, all is well. But the bug occurs in the <TT>if()</TT>
condition, because (contrary to what the programmer had thought)
this code is not testing if the item was found. The comparison is
actually testing if the iterator is not zero. Most iterators are
implemented as pointers, so this will compile (since a pointer can
be implicitly tested against zero.) But the meaning is flawed
because <TT>find()</TT> will always return an iterator that is not
"null".
</P>
<P>
<TT>find()</TT> returns an iterator pointing to the one-past-last element
in the sequence when the element is not found, equal to the iterator
returned by calling <TT>end()</TT>. We therefore must compare the returned
iterator against the container's <TT>end()</TT> value, and not against zero.
</p>
<P>
<TABLE WIDTH="75%">
<TR>
<TD VALIGN="top">
<IMG SRC="images/exclamation.gif" ALT="Note" BORDER=0 HSPACE="0" ALIGN="top" width="32" height="48">
</TD>
<TD valign="top">
<b>Note:</b>
<hr size = 1>
<tt> find(iter1, iter2, value);</tt> <br><br>
<TT>find</tt> returns <tt>iter2</TT> if value is not found. <tt>find</TT> does not return <tt>NULL</tt>. Don't
compare the return value of <TT>find</TT> with <TT>NULL</TT> or zero, because the comparison will never be true!
<hr size = 1>
</TD>
</TR>
</TABLE>
<P>
The code should be like this:
</P>
<pre>
V<b>:</b><b>:</b>iterator i <b>=</b> std<b>:</b><b>:</b>find<b>(</b>v1<b>.</b>begin<b>(</b><b>)</b><b>,</b> v1<b>.</b>end<b>(</b><b>)</b><b>,</b> a<b>)</b><b>;</b>
<b>if</b> <b>(</b>i <b>!=</b> v1<b>.</b>end<b>(</b><b>)</b><b>)</b> <font color="navy">// Note the change!!!</font>
<b>{</b>
v1<b>.</b>erase<b>(</b>i<b>)</b><b>;</b>
<b>}</b>
</pre>
<P>
Again, consider the possibility of leaking the object to which the
pointer being erased still points. We remove the element but don't
delete the pointer. We'll assume that <TT>v1[i]</TT> has other references so
it is not leaked by this operation.
</P>
<P>
<B>Question #5:</B>
</P>
<pre>
<font color="navy">// count all a's in the range that are "special"</font>
<b>extern</b> <b>bool</b> is_special_value<b>(</b>A <b>*</b><b>)</b><b>;</b>
<b>int</b> count_special_values<b>(</b>V <b>const</b> <b>&</b> v<b>)</b>
<b>{</b>
<b>int</b> count <b>=</b> <font color="blue">0</font><b>;</b>
V<b>:</b><b>:</b>iterator i <b>=</b> v<b>.</b>begin<b>(</b><b>)</b><b>;</b>
<b>while</b> <b>(</b>i <b>!=</b> v<b>.</b>end<b>(</b><b>)</b><b>)</b>
<b>{</b>
<b>if</b> <b>(</b>is_special_value<b>(</b><b>*</b>i<b>)</b><b>)</b>
<b>{</b>
<b>++</b>count<b>;</b>
<b>}</b>
i<b>++</b><b>;</b>
<b>}</b>
<b>return</b> count<b>;</b>
<b>}</b>
</pre>
<P>
Why doesn't this compile? Once it compiles, what can be improved
about this function?
</P>
<P>
<B>Answer #5:</B>
</P>
<P>
Actually, there are several things that can be improved here. First
is a minor efficiency detail, but it occurs frequently enough that
it is worth mentioning. In the for loop, the <TT>i</TT> is postfix
incremented (<TT>i++</TT>). That means that the old value is returned in a
temporary copy, and the iterator is also incremented. Because it
returns a temporary iterator that is not used, it is wasted
overhead. ALWAYS use prefix increment (<TT>++i</TT>) whenever the
⌨️ 快捷键说明
复制代码Ctrl + C
搜索代码Ctrl + F
全屏模式F11
增大字号Ctrl + =
减小字号Ctrl + -
显示快捷键?