Re: Find error
James Kanze wrote:
On Nov 30, 9:01 am, Kai-Uwe Bux <jkherci...@gmx.net> wrote:
yayalee1...@gmail.com wrote:
is there any error in the following code?
class PrettyMenu
{
public:
void changebackground(std::istream& new);
private:
mutex fmutex;
image *fimage;
int changenum;//record the change times
}
void changebackgroud(std::istream& new)
{
lock(&fmutex);
delete fimage;
++changnum;
fimage=new image(new);
unlock(&fmutex);
}
I am not going to open the standard for this one: even if the
code should turn out formally correct, it should be rewritten
anyway.
a) The use of "new" as the name for a parameter should be avoided,
especially if it leads to lines like
fimage = new image (new);
More than just "avoided". It's illegal, and will not compile.
b) The member fimage is a pointer for no reason. Apparently, you allocate
and copy-construct the image anyway (at least in the method shown, the
objects behaves as though it is the exclusive owner of the image), so the
pointer buys you nothing but trouble.
I don't see where he copies anything.
True, I was sloppy. What I was refering to is that the pointee is not shared
between several PrettyMenu objects: each object is the exclusive owner of
its image. In those cases, I just don't see any need for a pointer. Values
would do just fine. Of course, the PrettyMenu class might have semantics
that involves identity. However, I do not see why images should be like
that too. It seems to be perfectly reasonable that images should support
IO, copying and assignment.
If "image" doesn't
support copy and assignment, he needs a pointer.
No, he needs to change the image class so that is has copy constructor and
assignment operator :-)
If the names
here mean what they seem to mean, it's almost certain that image
has identity, and thus, it is highly probable that it doesn't
support copy and assignment.
Where do you get that meaning of "image"? I could see that for "PrettyMenu",
but not for "image".
[snip]
Best
Kai-Uwe Bux