what is wrong with this code?
-
i have a code from one of my past papers n it was asked to put the reason that what was wrong with that code. i looked on it very much but didnt find any problem. following is the code, kindly tell me even if there is any efficiency or any other drawback in this code. class A { int *pi; public: A() { pi = new int; } };
-
i have a code from one of my past papers n it was asked to put the reason that what was wrong with that code. i looked on it very much but didnt find any problem. following is the code, kindly tell me even if there is any efficiency or any other drawback in this code. class A { int *pi; public: A() { pi = new int; } };
int *pi;instead ofint* pi;{ pi = new int; }no parentheses, no return. :-D Short meaningless names? Unaligned braces? Not enough whitespace? No pre tags? I dunno. -
i have a code from one of my past papers n it was asked to put the reason that what was wrong with that code. i looked on it very much but didnt find any problem. following is the code, kindly tell me even if there is any efficiency or any other drawback in this code. class A { int *pi; public: A() { pi = new int; } };
You never delete the memory allocated with
new. :)If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler. -- Alfonso the Wise, 13th Century King of Castile.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong. -- Iain Clarke
[My articles] -
i have a code from one of my past papers n it was asked to put the reason that what was wrong with that code. i looked on it very much but didnt find any problem. following is the code, kindly tell me even if there is any efficiency or any other drawback in this code. class A { int *pi; public: A() { pi = new int; } };
-
i have a code from one of my past papers n it was asked to put the reason that what was wrong with that code. i looked on it very much but didnt find any problem. following is the code, kindly tell me even if there is any efficiency or any other drawback in this code. class A { int *pi; public: A() { pi = new int; } };
My original comment was misplaced. I suspect that the real problem may be that the variable
piis private and thus can never be accessed outside the class. So creating an object of classAserves no real purpose as you cannot set or get the value ofpi.I must get a clever new signature for 2011.
modified on Sunday, January 23, 2011 8:32 AM
-
i have a code from one of my past papers n it was asked to put the reason that what was wrong with that code. i looked on it very much but didnt find any problem. following is the code, kindly tell me even if there is any efficiency or any other drawback in this code. class A { int *pi; public: A() { pi = new int; } };
Aside from the fact that the code doesn't do anything useful, the really serious problems with this class are: - It will leak in normal operation (I use the term "operation" loosely here as the class doesn't do anything) - It isn't exception safe or neutral To fix these you're going to need a destructor, a copy constructor (or a private declaration of a copy constructor) and a swapping assignment operator (or a private declaration of an assignment operator). In addition (ignoring the questionable sanity of storing an int like that) the implementation could be tightened up to use an initialiser list. The idea here is to make a class that behaves like a object of a type built into the language. The structure your class needs to achieve this is known in some circles as "The Orthodox Canonical Form" and the first place I saw it mentioned and explained properly was in "Advanced C++: Programming Styles and Idioms" By Jim Coplien back in the early 90s. Other authors have picked up the term since then so a search for it might give you some more background. Cheers, Ash
-
Aside from the fact that the code doesn't do anything useful, the really serious problems with this class are: - It will leak in normal operation (I use the term "operation" loosely here as the class doesn't do anything) - It isn't exception safe or neutral To fix these you're going to need a destructor, a copy constructor (or a private declaration of a copy constructor) and a swapping assignment operator (or a private declaration of an assignment operator). In addition (ignoring the questionable sanity of storing an int like that) the implementation could be tightened up to use an initialiser list. The idea here is to make a class that behaves like a object of a type built into the language. The structure your class needs to achieve this is known in some circles as "The Orthodox Canonical Form" and the first place I saw it mentioned and explained properly was in "Advanced C++: Programming Styles and Idioms" By Jim Coplien back in the early 90s. Other authors have picked up the term since then so a search for it might give you some more background. Cheers, Ash
I support this answer, this is more reasonable.
Величие не Бога может быть недооценена.
-
Aside from the fact that the code doesn't do anything useful, the really serious problems with this class are: - It will leak in normal operation (I use the term "operation" loosely here as the class doesn't do anything) - It isn't exception safe or neutral To fix these you're going to need a destructor, a copy constructor (or a private declaration of a copy constructor) and a swapping assignment operator (or a private declaration of an assignment operator). In addition (ignoring the questionable sanity of storing an int like that) the implementation could be tightened up to use an initialiser list. The idea here is to make a class that behaves like a object of a type built into the language. The structure your class needs to achieve this is known in some circles as "The Orthodox Canonical Form" and the first place I saw it mentioned and explained properly was in "Advanced C++: Programming Styles and Idioms" By Jim Coplien back in the early 90s. Other authors have picked up the term since then so a search for it might give you some more background. Cheers, Ash
Oh, sure, if you want to get all technical. :rolleyes:
-
i have a code from one of my past papers n it was asked to put the reason that what was wrong with that code. i looked on it very much but didnt find any problem. following is the code, kindly tell me even if there is any efficiency or any other drawback in this code. class A { int *pi; public: A() { pi = new int; } };
Building on some of the already posed answers: As mentioned before, you need a destructor to make sure pi is released. Moreover, if you provide no Copy constructor or assignment operator, the compiler will create these for you. Both will simply copy pi over without checking, und thus also produce a memory leak. This means you need to define both a copy constructor and an assignment operator yourself, to prevent memory leaks from inadvertent copying! Another, minor, issue is that you didn't think to initialize the integer pi is pointing to. I will ignore the apparent uselessness of the class in it's current state, assuming that it's going to be expanded later. Going with the above, I would rewrite this to sth like that:
class A {
int* pi;
public:
int* getpi() {
if (0 == pi)
pi = new int();
if (pi != 0)
*pi = 0;
return pi;
}
A() : pi(0) {}
A(const A& other) : pi(0) {
*getpi() = *other.pi; /* deep copy */
}
~A() { delete pi; }
A& operator=(const A& other) {
*getpi() = *other.pi; /* deep copy */
return *this;
}
}Note that you could also implement a shallow copy by just redirecting pi to wherever the other object's pi is pointing, but if you do that, you need some mechanism such as reference counting to prevent deleting an int object while oter instances are still pointing to it! Note also that I only programmed the happy case with respect to allocation: if getpi() ever fails to allocate pi (and thus returns 0), then the call to the copy constructor or assignment operator will throw a NULL pointer exception. You might want to catch that, especially in the constructor. [edit]Made getpi() public to resolve the inaccessibility issue, and added initializion of *pi[/edit]
modified on Monday, January 31, 2011 6:02 AM