0

In this situation, I want to delete book, but I try declare this code, its not working. 

class Library {
private:
    Book **books;
    int counter;
public:
    Library() {
        books = NULL;
        counter = 0;
    }
    void Add(INPUT &tmp) {
        books = new Book*[counter];
        ++counter;
    }
    void Delete() {
        --counter;
        delete[] books[counter];
        books[counter] = NULL;
    }
    int getCounter() {
        return this->counter;
    }
    ~Library() {
        delete[] books;
    }
};
mstfyvtrl
  • 9
  • 4
  • what does not work? If you get compiler errors please include them in the question. If it crashes during runtime please let us know (and note that the code you show here is just a class definition, so there is nothing to run here). – 463035818_is_not_a_number Sep 07 '18 at 13:37
  • 1
    1) Please explain what, exactly, doesn't work for you. 2) Please provide [mcve]. – Algirdas Preidžius Sep 07 '18 at 13:37
  • You need to add more information. Why doesn't it work? Will it not compile? Does it throw an error message? There are a number of problems with your code, but it's impossible to know which is the root causative factor with respect to the specific problem you're encountering. – Scott Mudge Sep 07 '18 at 13:38
  • @JeJo "_`Book **books;` this does not look like a C++ code._" Why? Double pointers are as valid in C++, as in C. – Algirdas Preidžius Sep 07 '18 at 13:38
  • the increments and decrements of your counter look fishy. The first time you add something `counter == 0` so you create a 0 sized array and only then increment the counter. – 463035818_is_not_a_number Sep 07 '18 at 13:38
  • error message : C:\Users\User\source\repos\Project2\Debug\Project2.exe (process 12648) exited with code -1073741819. – mstfyvtrl Sep 07 '18 at 13:42
  • @Jejo Yes, I agree that one should use `std::vector` wherever possible instead of raw pointers, however stating that it's invalid C++, is just wrong. – Algirdas Preidžius Sep 07 '18 at 13:43
  • 1
    @MustafayevTural Did you try stepping through your code with a debugger? – Algirdas Preidžius Sep 07 '18 at 13:43
  • @AlgirdasPreidžius yes i did: Unhandled exception at 0x00865BC2 in Project2.exe: 0xC0000005: Access violation reading location 0xFDFDFDF9. – mstfyvtrl Sep 07 '18 at 13:46
  • 1
    @MustafayevTural This doesn't answer my question. Let me expand, on my initial comment, about what does it mean to "step through your code with a debugger": Did you try stepping through your code, line-by-line, while investigating the values of local variables, at each step, while, simultaneously, comparing them with your expectations, to see the point where the behavior of your program, doesn't match your expectations? – Algirdas Preidžius Sep 07 '18 at 13:52
  • The biggest mistake (there are quite a few mistakes) is using a double pointer. A library is a collection of Books, you only need a single pointer for that, `Book *books;`. Fix that then start writing your code again. – john Sep 07 '18 at 14:00
  • 1
    @john Well, we don't know what `Book` is in OP's code, nor how it should be used. They may need a `std::vector>` for what we know. – Bob__ Sep 07 '18 at 15:00

2 Answers2

3

Before you get deleting to work, you need to get adding right.

In addition to what Jeffrey said, your Add function probably doesn't work correctly due to an "out by one" error. In the first call you will have books = new Book*[0];. Allocating a zero sized array is legal (see here) but you will not be able to store anything in it.

If you can use a std::vector it will make your code much simpler and less error prone.

class Library {
private:
    std::vector<Book> books;
    // no need for counter, std::vector has size()
public:
    // no need for a constructor, the default constructor
    // will correctly construct 'books'
    void Add(INPUT &tmp) {
        // not sure how you convert 'INPUT' to 'Book'
        books.push_back(tmp);
        // this handles all of the memory management for you
    }
    void Delete() {
        // you need to ensure that books is not empty
        books.pop_back();
    }
    int getCounter() {
        return books.size();
    }
    // no need for a destructor, the default one will
    // do everything
};

If you need two dimensions, then the code is similar but will use a vector of vector.

Lightness Races in Orbit
  • 358,771
  • 68
  • 593
  • 989
Paul Floyd
  • 3,765
  • 5
  • 25
  • 37
  • Perhaps `Book book(tmp); books.push_back(book);`. – Eljay Sep 07 '18 at 14:12
  • 1
    @Eljay I raise with [`books.emplace_back(tmp);`](https://en.cppreference.com/w/cpp/container/vector/emplace_back)... – Bob__ Sep 07 '18 at 14:49
  • @Bob • but `tmp` is not a Book, it is an INPUT. – Eljay Sep 07 '18 at 18:24
  • 1
    @Eljay Exactly. In your snippet you suppose there's a constructor of `Book` accepting an `INPUT` and what `emplace_back` does is to construct the element in-place using the provided argument, without a temporary. – Bob__ Sep 07 '18 at 18:31
1
Book **books;

is a pointer to a pointer to a book. It is the old style way of having a list of books (as pointers) or a list of list of books.

Library() {
    books = NULL;
    counter = 0;
}

This will create an empty library. No books.

void Add(INPUT &tmp) {
    books = new Book*[counter];
    ++counter;
}

First thing to notice is that you are not using the tmp book. So you probably won't succeed in storing it anywhere, without using it.

Second thing is that books = new Book*[counter]; allocates a library of books. Space for storing some books. You probably should do that in the constructor. If you do it there, every time you try to add a book, you'll lose all the others, and you'll also leak memory.

There's two possibilities here. You have an old-timer C++ professor, and you'll need to learn about pointers and pointers to pointers, and new, delete. Or you could learn about std::vectors and smart-pointers. This would be a better idea, but I can't tell you how well it will be received in your class.

Also, please state what INPUT is defined as.

Jeffrey
  • 7,814
  • 1
  • 16
  • 32