0

for my homework i am supposed to change the vector from the registry class to an array and define it in the BankVector class. I did so andthe errormessage in the compiler is really... confusing. Im new with pointers soI am really confused about this. What is the mistake ? or are the mistakes haha SEND HELP PLEASE

#include <iostream>
#include <string>
#include <array>
#include <fstream>
using namespace std; 

class Bankaccount{

  private: 
    string accName; 
    double balance;  
    char type;

  public:
    Bankaccount(); 
    Bankaccount(string name, char credit);
    string getName(); 
    double getBalance(); 
    void setBalance(double transferMoney); 
    char getCredit(); 

}; 

Bankaccount::Bankaccount(){
  accName = " ";
  balance = 0; 
  type = 'n'; 
}

Bankaccount::Bankaccount(string name, char credit){
  accName = name; 
  balance = 0; 
  type = credit;
}

string Bankaccount::getName(){
  return accName; 
}

double Bankaccount::getBalance(){
  return balance; 
}

void Bankaccount::setBalance(double transferMoney){
  balance += transferMoney; 
}

char Bankaccount::getCredit(){
  return type;
}



class BankVector {

  private:
        Bankaccount* entries;
        unsigned int usedCapacity;      
        unsigned int maxCapacity;

    public:
        BankVector();
        ~BankVector();
        unsigned int size() const;
        Bankaccount& at(unsigned int index) const;
        void push_back(Bankaccount a);
        void erase(unsigned int index);

        class IndexError:public exception{

          const char* what() const throw(){ 
            return "error: Index out of range";
          }
    };

};

BankVector::BankVector(){
    entries = new Bankaccount;
    usedCapacity = 0; 
  maxCapacity = 0; 
}

BankVector::~BankVector(){
     delete[] entries; 
}

unsigned int BankVector::size() const{
    return usedCapacity;
}

Bankaccount& BankVector::at(unsigned int index) const{
  if(index > usedCapacity){
    throw IndexError();
  } else {
    return entries[index];
  }
}

void BankVector::push_back(Bankaccount a){
    if(usedCapacity == maxCapacity){
      Bankaccount* tmp = new Bankaccount[maxCapacity + 1];
    for(int i = 0; i < maxCapacity; i++){
      tmp[i] = entries[i];
    }
    delete entries; 
    for(int i = 0; i < maxCapacity; i++){
     entries[i] = tmp[i];
    }
    delete tmp; 
    entries[maxCapacity] = a; 
    } else {
      entries[usedCapacity + 1] = a;  
    }
}

void BankVector::erase(unsigned int index){
    if(index >= 0 && index < usedCapacity){
        for(int i = index; i < usedCapacity - 1; i++){
            entries[i] = entries[i + 1]; 
        }
    usedCapacity = usedCapacity - 1; 
    } else{
      throw IndexError(); 
    }
}


class Registry{
  public: 
    void addAcc(string name, char isCredit); 
    void removeAcc(string name); 
    void transaction(string name, double transMoney); 
    bool checkDupl(string name); 
    double checkBalance(string name);
    char checkCreditType(string name); 
    void print();

  private: 
    BankVector accountlist; 
    int i; 
    double balance = 0;
    char type; 

}; 

void Registry:: addAcc(string name, char isCredit){
  Bankaccount newAcc(name, isCredit); 
  accountlist.push_back(newAcc); 
}

void Registry::removeAcc(string name){
  for(i = 0; i < accountlist.size(); i++){
    if(accountlist.at(i).getName() == name){
      accountlist.erase(i); 
    }
  }
}

void Registry::transaction(string name, double transMoney){
  for(i = 0; i < accountlist.size(); i++){
    if(accountlist.at(i).getName() == name){
      accountlist.at(i).setBalance(transMoney); 
    }
  }
}

bool Registry::checkDupl(string name){
  for(i = 0; i < accountlist.size(); i++){
    if(accountlist.at(i).getName() == name){
      return true; 
    }
  }
  return false; 
}

double Registry::checkBalance(string name){
    for(i = 0; i < accountlist.size(); i++){
      if(accountlist.at(i).getName() == name){
        balance = accountlist.at(i).getBalance();
      }
    }
  return balance;
}

char Registry::checkCreditType(string name){
  for(i = 0; i < accountlist.size(); i++){
      if(accountlist.at(i).getName() == name){
        type = accountlist.at(i).getCredit();
      }
    }
  return type;
}

void Registry::print(){
  for(i = 0; i < accountlist.size(); i++){
    if(accountlist.at(i).getBalance() >= 0){
      cout << accountlist.at(i).getName() << " owns " << accountlist.at(i).getBalance() << " euros" << endl; 
    } else if(accountlist.at(i).getBalance() < 0){
      cout << accountlist.at(i).getName() << " owes " << accountlist.at(i).getBalance() << " euros" << endl; 
    }
  }
}


int main(){
  Registry account;
  ifstream inFS; 
  string file, command, fileName;
  char creditType; 
  double moneyVal = 0; 
  int i = 0; 

  cout << "Enter the name of the records file: ";
  cin >> file; 

  inFS.open(file); 
  if(!inFS.is_open()){
    cout << "Could not open file " << file;
    return 0;
  }

  while(!inFS.eof()){
    i++;

    try{

      inFS >> command >> fileName; 

      if(command == "c"){
        inFS >> creditType;
        if(account.checkDupl(fileName) == true){
          throw runtime_error(": account already exists"); 
        } else {
          account.addAcc(fileName, creditType); 
        }
      }

      if(command == "r"){
        if(account.checkDupl(fileName) == false){
          throw runtime_error(": account does not exist");
        } else if(account.checkDupl(fileName) == true){
          if(account.checkBalance(fileName) >= 0){
            account.removeAcc(fileName);
          } else if(account.checkBalance(fileName) < 0){
            throw runtime_error(": account holds negative balance"); 
          }
        }
      }

      if(command == "t"){
        inFS >> moneyVal;
        if(account.checkDupl(fileName) == true){
          if((account.checkBalance(fileName) + moneyVal) >= 0){
            account.transaction(fileName, moneyVal);  
          } else if((account.checkBalance(fileName) + moneyVal) < 0){
            if(account.checkCreditType(fileName) == 'y'){
              account.transaction(fileName, moneyVal);  
            } else if(account.checkCreditType(fileName) == 'n'){
              throw runtime_error(": account cannot hold negative balance"); 
            }
          }
        }else{
          throw runtime_error(": account does not exist");
        }
      }  
    }

    catch(runtime_error& excpt){
      cout << "error on line " << to_string(i) << excpt.what() << endl; 
    }
  }

  cout<< endl; 
  account.print();

  return 0; 
}

Thas what the compiler prints:

What is does that bug say: Enter the name of the records file:  records1.bank 

*** Error in `./main': free(): invalid pointer: 0x0000000001a466c8 *** ======= Backtrace: ========= /lib/x86_64-linux-gnu/libc.so.6(+0x70bfb)[0x7f1e40109bfb] /lib/x86_64-linux-gnu/libc.so.6(+0x76fc6)[0x7f1e4010ffc6] /lib/x86_64-linux-gnu/libc.so.6(+0x7780e)[0x7f1e4011080e] ./main[0x402466] ./main[0x402685] ./main[0x402f28] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf1)[0x7f1e400b92e1] ./main[0x401f8a] ======= Memory map: ======== 00400000-00405000 r-xp 00000000 08:01 11032506                           /home/runner/main 00604000-00605000 r--p 00004000 08:01 11032506                           /home/runner/main 00605000-00606000 rw-p 00005000 08:01 11032506                           /home/runner/main 01a32000-01a64000 rw-p 00000000 00:00 0                                  [heap] 7f1e3c000000-7f1e3c021000 rw-p 00000000 00:00 0  7f1e3c021000-7f1e40000000 ---p 00000000 00:00 0  7f1e40099000-7f1e4022e000 r-xp 00000000 08:01 2562779                    /lib/x86_64-linux-gnu/libc-2.24.so 7f1e4022e000-7f1e4042e000 ---p 00195000 08:01 2562779                    /lib/x86_64-linux-gnu/libc-2.24.so 7f1e4042e000-7f1e40432000 r--p 00195000 08:01 2562779                    /lib/x86_64-linux-gnu/libc-2.24.so 7f1e40432000-7f1e40434000 rw-p 00199000 08:01 2562779                    /lib/x86_64-linux-gnu/libc-2.24.so 7f1e40434000-7f1e40438000 rw-p 00000000 00:00 0  7f1e40438000-7f1e40450000 r-xp 00000000 08:01 2562842                    /lib/x86_64-linux-gnu/libpthread-2.24.so 7f1e40450000-7f1e4064f000 ---p 00018000 08:01 2562842                    /lib/x86_64-linux-gnu/libpthread-2.24.so 7f1e4064f000-7f1e40650000 r--p 00017000 08:01 2562842                    /lib/x86_64-linux-gnu/libpthread-2.24.so 7f1e40650000-7f1e40651000 rw-p 00018000 08:01 2562842                    /lib/x86_64-linux-gnu/libpthread-2.24.so 7f1e40651000-7f1e40655000 rw-p 00000000 00:00 0  7f1e40655000-7f1e4066c000 r-xp 00000000 08:01 2574182                    /usr/local/lib64/libgcc_s.so.1 7f1e4066c000-7f1e4086b000 ---p 00017000 08:01 2574182                    /usr/local/lib64/libgcc_s.so.1 7f1e4086b000-7f1e4086c000 r--p 00016000 08:01 2574182                    /usr/local/lib64/libgcc_s.so.1 7f1e4086c000-7f1e4086d000 rw-p 00017000 08:01 2574182                    /usr/local/lib64/libgcc_s.so.1 7f1e4086d000-7f1e40970000 r-xp 00000000 08:01 2562808                    /lib/x86_64-linux-gnu/libm-2.24.so 7f1e40970000-7f1e40b6f000 ---p 00103000 08:01 2562808                    /lib/x86_64-linux-gnu/libm-2.24.so 7f1e40b6f000-7f1e40b70000 r--p 00102000 08:01 2562808                    /lib/x86_64-linux-gnu/libm-2.24.so 7f1e40b70000-7f1e40b71000 rw-p 00103000 08:01 2562808                    /lib/x86_64-linux-gnu/libm-2.24.so 7f1e40b71000-7f1e40ce4000 r-xp 00000000 08:01 2574242                    /usr/local/lib64/libstdc++.so.6.0.25 7f1e40ce4000-7f1e40ee4000 ---p 00173000 08:01 2574242                    /usr/local/lib64/libstdc++.so.6.0.25 7f1e40ee4000-7f1e40eee000 r--p 00173000 08:01 2574242                    /usr/local/lib64/libstdc++.so.6.0.25 7f1e40eee000-7f1e40ef0000 rw-p 0017d000 08:01 2574242                    /usr/local/lib64/libstdc++.so.6.0.25 7f1e40ef0000-7f1e40ef3000 rw-p 00000000 00:00 0  7f1e40ef3000-7f1e40f16000 r-xp 00000000 08:01 2562761                    /lib/x86_64-linux-gnu/ld-2.24.so 7f1e41104000-7f1e41109000 rw-p 00000000 00:00 0  7f1e41112000-7f1e41116000 rw-p 00000000 00:00 0  7f1e41116000-7f1e41117000 r--p 00023000 08:01 2562761                    /lib/x86_64-linux-gnu/ld-2.24.so 7f1e41117000-7f1e41118000 rw-p 00024000 08:01 2562761                    /lib/x86_64-linux-gnu/ld-2.24.so 7f1e41118000-7f1e41119000 rw-p 00000000 00:00 0  7ffcdc330000-7ffcdc351000 rw-p 00000000 00:00 0                          [stack] 7ffcdc360000-7ffcdc363000 r--p 00000000 00:00 0                          [vvar] 7ffcdc363000-7ffcdc365000 r-xp 00000000 00:00 0                          [vdso] ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0                  [vsyscall] exit status -1   
Thesa97
  • 23
  • 1
  • 2
    It says that you try to `delete` (or `delete[]`) a pointer that wasn't previously allocated by `new` (or `new[]`) or perhaps was `delete`d twice. – Swordfish Oct 22 '18 at 21:06
  • 5
    Just looked at your code ... pointers allocated with `new` must be deallocated using `delete` and pointers allocated with `new[]` must be deallocated with `delete[]`. There is no mix and match. – Swordfish Oct 22 '18 at 21:07
  • 2
    And you should read about the [Rule of 3/5/0](https://en.cppreference.com/w/cpp/language/rule_of_three) – Swordfish Oct 22 '18 at 21:09
  • In the default constructor of `BankVector` you allocate memory with `new` but the destructor uses `delete[]` to deallocate it. Also, there is no need to allocate memory in the default constructor since there is no data to be managed. (`maxCapacity == usedCapacity == 0`) btw. you should use the constructors initialization list to initialize members. – Swordfish Oct 22 '18 at 21:12
  • 1
    These assignments should just have you create your own vector class so that you learn the basics of how one is put together. – PaulMcKenzie Oct 22 '18 at 21:17
  • Your `push_back` never updates `usedCapacity`. Function `at()`: `if(index > usedCapacity)` should be `if(index >= usedCapacity)` – Swordfish Oct 22 '18 at 21:18
  • I recommend you enable the compiler to help you. I found [-Wsign-compare] several places ; [-Wtype-limits] 1 place (variable index) ; [-Wsign-conversion] 1 place ... suggest you add these warnings to your compile command, and fix the error (int, uint, etc) – 2785528 Oct 22 '18 at 21:33

2 Answers2

1

Let's break down the guts of your push_back function:

Bankaccount* tmp = new Bankaccount[maxCapacity + 1];
for(int i = 0; i < maxCapacity; i++){
    tmp[i] = entries[i];
}

So far, so good. You have created a new array with one extra space. (bonus: read about why this is a bad idea)

delete entries;

Okay, so you don't need the old array, but as already highlighted in the comments section, you must use delete[] entries; since it was allocated with new[].

for(int i = 0; i < maxCapacity; i++){
    entries[i] = tmp[i];
}

Here's where it gets wild. You just deleted entries, but now you are dereferencing an invalid pointer and writing to memory. What you actually wanted to do is replace that entire loop with entries = tmp;. That just discards the old pointer that's no longer valid, and stores the new one that is valid.

delete tmp; 

NOOOOO! Not only should you have used delete[], but you don't want to do this. That just destroyed the last remaining copy of your array. Now where is your data? Remove this line.

entries[maxCapacity] = a; 

Yeah it sort of works, but you forgot one thing: maxCapacity should have been incremented when you resized the array (or at least somewhere). This line is technically okay, but semantically incorrect. You have another variable usedCapacity which tells you where to store the data. In theory, you should be increasing maxCapacity by some factor each time you resize, which means it is generally not pointing to the next free slot. So you should do this instead:

    // Better approach...
    entries[usedCapacity] = a;
    ++usedCapacity;
    ++maxCapacity;  // <- this line should actually go earlier when you resize

I hope this gets you started. There are other mistakes. For example, you use the wrong new in the BankVector's constructor. Actually, you should not allocate at all. The capacity starts at zero, so just set the pointer to NULL.

When in doubt, step through your code with a debugger.

paddy
  • 60,864
  • 6
  • 61
  • 103
-2

There are several blunders in your BankVector class. Here is a somewhat cleaned-up and (hopefully) correct version with member methods inlined for clarity.

class BankVector {
    Bankaccount* entries = nullptr;    // provide default initialisers
    unsigned int usedCapacity = 0;      
    unsigned int maxCapacity = 0;

    void grow()                        // double maxCapacity
    {
        maxCapacity = maxCapacity? 2*maxCapacity : 1;
        Bankaccount*tmp = new Bankaccount[maxCapacity];
        for(unsigned int i=0; i!=usedCapacity; ++i)
            tmp[i] = entries[i];
        delete[] entries;
        entries = tmp;
    }

  public:
    BankVector() = default;            // use default initialisers

    ~BankVector()
    { delete[] entries; }

    unsigned int size() const
    { return usedCapacity; }

    unsigned int capacity() const
    { return maxCapacity; }

    Bankaccount const& at(unsigned int index) const   // const access
    {
        if(index >= size())
            throw std::range_error("BankVector::at(): index=" +
                std::to_string(index) + " >= size()=" + std::to_string(size()));
        return entries[index];
    }

    Bankaccount & at(unsigned int index)   // non-const access
    {
        if(index >= size())
            throw std::range_error("BankVector::at(): index=" +
                std::to_string(index) + " >= size()=" + std::to_string(size()));
        return entries[index];
    }

    void push_back(Bankaccount const&a)
    {
        if(size() >= capacity())
            grow();
        entries[usedCapacity++] = a;
    }

    void remove(unsigned int index)    // fast but not order-preserving
    {
        if(index < usedCapacity)
            entries[index]=entries[--usedCapacity];
    }

    void erase(unsigned int index)     // order-preserving but slow
    {
        if(index >= size())
            throw std::range_error("BankVector::erase(): index=" +
                std::to_string(index) + " >= size()=" + std::to_string(size()));
        usedCapacity--;
        for(unsigned int i=index; i!=usedCapacity; ++i)
            entries[i] = entries[i+1];
    }
};
Walter
  • 44,150
  • 20
  • 113
  • 196
  • There are both syntax and logic errors in this answer. In general, I think it's preferable to explain what the OP did wrong and have them correct their mistake, rather than simply rewrite their code for them. – paddy Oct 22 '18 at 21:47
  • `erase()` wt .. heck? – Swordfish Oct 22 '18 at 21:49
  • `if(size() > capacity())` when this is ever true, your vector implementation is broken. – Swordfish Oct 22 '18 at 21:49
  • @Swordfish indeed. fixed. – Walter Oct 22 '18 at 21:51
  • 1
    We're going to spend more time correcting this answer than correcting the original code. – paddy Oct 22 '18 at 21:51
  • @Walter Nothing fixed. paddy+1 – Swordfish Oct 22 '18 at 21:52
  • @swordfish would you be inclined to be more detailed in your comments, please? – Walter Oct 22 '18 at 21:54
  • Could the downvoter please *specify* the reasons for downvoting? – Walter Oct 22 '18 at 21:58
  • I didn't downvote (yet) but do you really need to ask for a reason when your code is broken?? – Swordfish Oct 22 '18 at 21:59
  • "would you be inclined to be more detailed in your comments, please?" Missing parantheses after `unsigned int capacity`. In `at()`: `if(index > size())` wrong ==> `if(index >= size())`. `erase()`: overwriting an element is hardly the same as erasing it. `unsigned int` should really be `std::size_t`. – Swordfish Oct 22 '18 at 22:06
  • 1
    I downvoted because I felt the answer is not useful. It neither answers the question, nor teaches the OP anything. Your code won't work or even compile despite your current efforts to fix it. Finally, it adds syntax from more recent language editions, which are not evident in the original code. All this only serves to confuse the OP, instead of educate them. – paddy Oct 22 '18 at 22:06
  • @swordfish overwriting a primitive type is no different from erasing it; `unsigned int` was the OP's choice and should really be `ptrdiff_t` (the choice of unsigned types for sizes and especially indices is a design flaw of the STL); – Walter Oct 22 '18 at 22:15
  • @paddy Nobody these days should *learn* pre 2011 C++. Even old software companies (such as google) use that standard (though watered down a bit by not using exceptions, for example). – Walter Oct 22 '18 at 22:17
  • "overwriting a primitive type is no different from erasing it;" `erase()` should REMOVE an element from the vector!! I won't engage with you in a discussion about signed/unsigned for sizes of objects or indexes. I'll just wish you much fun checking every passed index and size in your vector for `>= 0` if you use `ptrdiff_t` – Swordfish Oct 22 '18 at 22:22
  • @Swordfish my `erase()` does remove an element, that with index `index` by replacing/overwriting it with the last element and decrementing the size by one. There is no need to call a destructor for a primitive type. – Walter Oct 22 '18 at 22:24
  • @swordfish There is no need to check for `index>=0`, except in `at()`. When using an unsigned type, the problem does not disappear, as the numbers simply wrap (when obtaining an index from a difference evaluating to < 0). – Walter Oct 22 '18 at 22:28
  • Rename your `erase()` to `erase_and_shuffle()` then. "(when obtaining an index from a difference evaluating to < 0)" thats the users problem. "There is no need to check for index>=0, except in at()" Mhm. And in `erase()` youd happily accept negative indexes. – Swordfish Oct 22 '18 at 22:31
  • @swordfish `erase()` cannot be implemented without changing the map from indices to elements (unless the last element is erased), so there is no point in preserving the order of the remaining elements. – Walter Oct 22 '18 at 22:33
  • "there is no point in preserving the order of the remaining elements." The standard thinks otherwise for `std::vector`. And I'd consider a dynamic array that reorders ements on `erase()` broken. – Swordfish Oct 22 '18 at 22:49
  • @swordfish **1** I did amend my post (prior to your last comment) **2** The standard (2011) does not explicitly specify that `erase` (for sequence containers) preserves the order of the remaining elements (though this is obviously the implied intention). I wonder why there is no allowance made for a similar method (say `remove`), which costs only O(1), even for `vector`, but does not preserve the order? – Walter Oct 22 '18 at 23:04