Re: Find error
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. If "image" doesn't
support copy and assignment, he needs a pointer. 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.
c) The comment for the int is misleading: it makes you think
that the int stores a time stamp.
What about:
class PrettyMenu
{
public:
void changebackground(std::istream& istr);
private:
mutex fmutex;
image fimage;
int changenum;// count the changes
}
void changebackgroud(std::istream& istr)
{
lock(&fmutex);
istr >> fimage;
++changnum;
unlock(&fmutex);
}
If image supports it, fine.
If image doesn't, of course, his code (even correcting the name
new) doesn't work. He needs something like:
void
PrettyMenu::changebackgroud(
std::istream& newBackground )
{
image* tmp = new image( newBackground ) ;
lock( &myMutex ) ;
delete myBackground ;
myBackground = tmp ;
unlock( &myMutex ) ;
}
(I've changed the name of the pointer variable to something more
intelligent as well.)
Of course, a scoped lock of some sort would be preferable as
well (and necessary if the new is in the locked area).
--
James Kanze (GABI Software) email:james.kanze@gmail.com
Conseils en informatique orient=E9e objet/
Beratung in objektorientierter Datenverarbeitung
9 place S=E9mard, 78210 St.-Cyr-l'=C9cole, France, +33 (0)1 30 23 00 34