Hello, comrades, programmers! What problems do you see in this code?

class Foo { public: Foo(int j) { i=new int[j]; } ~Foo() { delete i; } private: int* i; }; class Bar: Foo { public: Bar(int j) { i=new char[j]; } ~Bar() { delete i; } private: char* i; }; void main() { Foo* f=new Foo(100); Foo* b=new Bar(200); *f=*b; delete f; delete b; } 
  • one
    No comments describing the idea of ​​this program. - avp 2:46 pm
  • 3
    It is written in C ++ =) - Specter
  • 2
    @Spectre, do not transfer your complexes to others - skegg
  • five
    And what is wrong with the disclosure of the fact that this is a test task for Yandex? - karmadro4
  • 2
    So I wonder why there was such a hype around this and why it was hidden :-) I do not see anything particularly shameful in collaboration with them. - karmadro4

6 answers 6

From what is not stated above:

What problems do you see in this code?

Absolutely meaningless code. Classes have a meaningless name and do nothing, because they have no interface. Bar has 2 members with the meaningless name i, in which you can get confused and think that this is one pointer. Also, the size of the array is not stored anywhere.

So far I have noticed only a memory leak in the place where the pointer is copied ...

You do not copy pointers to objects, but objects themselves. This leads to clipping, that is, only half of the Bar object is copied, namely the pointer to an int. In this case, both objects begin to refer to the same piece of memory, and the memory allocated by the Foo object flows away. When f is deleted, the array data is released, and b contains a dangling pointer in the part that is inherited from Foo. The second release of this block of memory leads to unpredictable consequences.

Thus, the code consists of continuous flaws. He does absolutely nothing and cannot be used in any way. If you want a static array, create it on the stack, if you want a dynamic array, select through new without intermediaries or use containers.

  • @GLmonster, you probably can explain. With a new Bar (200), which base foo will be created for it? In a sense, the constructor Foo will be invoked, and if so, with what argument? - avp
  • Well, what will be able to find. Most likely I won't be able to do anything (I tried to translate, g ++ explained to me popularly: error: no matching function for call to 'Foo::Foo()' - alexlz
  • By the way, yes, did not pay attention. The constructor Foo (int) will not be called. The base class constructor with arguments must be called explicitly in the initialization list. Otherwise, the constructor is called without arguments. Since the Foo class has a constructor with arguments, the compiler does not generate a default constructor. Therefore there will be a compilation error. - gammaker
  • ** Here !!! ** And I thought I did not know the crosses at all. The code is just wrong and there is nothing to break your head about leaks and what will happen in the destructor, etc. - avp
  • 3
    As in the old joke: - Well, how is the new apartment? Shortcomings a lot? - Yes, while one? -- It's good. Which one? - The front door does not open. - alexlz

Perhaps you should write like this:

 int *i = new int[50]; delete[] i; 

With such an entry, the memory allocated to the array is returned to the heap:

 int *i = new int[50]; delete i; 

Still in this code, a pointer is declared to a class that contains a single member, and the member itself is a pointer to an array of integers whose length is specified through the constructor. This alone is enough to discourage the desire to understand this C ++ code from a normal person.

  • Well, if you pay attention to such trifles, then, yes, this is also a bug. Thank. But still I would like to know about the bugs more serious. - AseN
  • This is also not particularly scary, but problematic. It is hardly possible to pass a parameter to a constructor. =)> In this code, a pointer to a class that contains a single member is declared, and this member itself is a pointer to an array of integers whose length is specified through the constructor. - AseN
  • one
    > With such a record, the memory allocated to the array is not returned to the heap. for the first. That is, for this case (built-in types) it is just a matter of style. - Jofsey

No class of inheritance from the class Bar. For example, class Bar: public Foo {...}; In an inherited class, the variable name is the same as the variable in the base class.

  • Well, this nonsense is easily solved: for this it is enough to make the variable i virtual in the parent class. - AseN

I would point out one big problem mentioned by @igumnov , namely, memory allocation depending on the constructor parameter. So you can easily demolish the head of any computer, it’s enough to call on a 64-bit machine:

 Foo* f=new Foo(INT_MAX); 

and everything will fall.

I would still put a fool proof test in the allocation code - for cons, for max. array size and so on.

  • > I would still type in the allocation code a test for fool resistance - for minuses, for max. array size and so on. This should usually be done with code that may be useful. To test for foolproof, simply remove all the code above, leaving an empty main. - gammaker

The name of the variables tells me nothing. Read chapter 11 (The Power of Variable Names) from the Perfect Code Book.

  • Absolutely agree. - AseN
  • one
    Just in time for pritenzy no. @ Anton Feoktistov, you don’t need to invent "meaningful" names for the code that checks the level of the responder. And then they confuse him (the respondent) even more. - avp
  • Are you from the point of view do not need, or just specifically given okoda? - Anton Feoktistov
  • 3
    This is a test task that has no relation to real work, therefore, the remark is completely out of place. - skegg

So, what errors actually exist in this code:

Well, you can start by saying that the code will not run at all at the compilation stage! The main function (main) returns nothing ( void main() ), but must return "int" => ( int main() ).


Error in Bar constructor.

To build an object, the base class constructor must be called. Since it is not specifically stated which constructor should be called, the compiler tries to substitute the default constructor call "Foo::Foo()" , but this is not defined in the class "Foo"


In the constructors of both classes there is no check for the negative of the variable "j" , as a result of which the constructor will try to create an array with a negative dimension, and this is a disaster ...

Fourth (memory leaks):

There are two (!) Memory leaks in this code! The first is obvious, the pointer is copied. But the second leak is hidden in the call "delete b" . This call will remove the object *b , but will only cause the destructor of the Foo-parts of the object *b , since the destructor is not virtual. Result: re-freeing the memory *b

Conclusion: a solid hole, not a code ... an error in almost every line.

 Тестировалось в компиляторе gcc 4.6 . 
  • one
    The latest version is what? Can you clarify? - skegg
  • Not, not the last, as it turned out (the last thought) 4.6 - AseN
  • > There are two (!) Memory leaks in this code! The first is obvious, the pointer is copied. OBJECTS are copied, and with them, and pointers inside them. > The main function (main) returns nothing (void main ()), but must return "int" In some compilers, you can write like this. For example, in Visual C ++. - gammaker