Simon
Simon

Reputation: 13

Function removes too much

I'm doing a phone registry and in it you need to be able to add, remove and show the phones on stock. I've made it possible to add in phones but whenever I add let's say 3 phones and remove the second one then both the third and second phone are deleted and I don't understand why.

This is my CellPhoneHandler.h file:

#ifndef CELLPHONEHANDLER_H
#define CELLPHONEHANDLER_H

#include "CellPhone.h"

class CellPhoneHandler
{
private:
    CellPhone **phone;
    int nrOfPhones;
    int priceOfPhone;
    int stockCapacity;
    int nrOfPhonesInArr;
public:
    CellPhoneHandler();
    ~CellPhoneHandler();
    void addPhone(string brand, int nrOf, int price);
    bool removePhoneFromStock(string name, int nrOf);
    int getNrOfPhones() const;
    int getNrOfPhonesInArr() const;
    int getPrice() const;
    void getPhonesAsString(string arr[], int nrOf, int priceOfPhone) const;
};

#endif // !CELLPHONEHANDLER_H

this is my CellPhoneHandler.cpp file.

#include "CellPhoneHandler.h"

CellPhoneHandler::CellPhoneHandler()
{
    this->phone = nullptr;
    this->nrOfPhones = 0;
    this->priceOfPhone = 0;
    this->stockCapacity = 0;
    this->nrOfPhonesInArr = 0;
}
CellPhoneHandler::~CellPhoneHandler()
{ 
    for (int i = 0; i < nrOfPhonesInArr; i++)
    {
        delete phone[i];
    }
    delete[] phone;
}
void CellPhoneHandler::addPhone(string brand, int nrOf, int price)
{
    if (stockCapacity < nrOfPhonesInArr + 1)
    {
        CellPhone ** tempArray = new CellPhone*[this->nrOfPhonesInArr + 1];

        for (int i = 0; i < nrOfPhonesInArr; i++)
        {
            tempArray[i] = this->phone[i];
        }

        delete[] this->phone;
        this->phone = tempArray;
        this->phone[this->nrOfPhonesInArr] = new CellPhone(brand, nrOf, price);
        this->nrOfPhonesInArr++;
        //this->stockCapacity++;
    }
}
bool CellPhoneHandler::removePhoneFromStock(string name, int nrOf)
{
    bool phoneFound = false;
    int index = nrOfPhonesInArr;

    for (int i = 0; i < nrOfPhonesInArr; i++)
    {
        if (this->phone[i]->getBrand() == name);
        {
            index = i;
            phoneFound = true;
            this->nrOfPhonesInArr--;            
        }
    }

    if (phoneFound == true)
    {
        delete phone[index];
        phone[index] = nullptr;
    }

    return phoneFound;
}
int CellPhoneHandler::getNrOfPhones() const
{
    return this->nrOfPhones;
}
int CellPhoneHandler::getNrOfPhonesInArr() const
{
    return this->nrOfPhonesInArr;
}
int CellPhoneHandler::getPrice() const
{
    return this->priceOfPhone;
}
void CellPhoneHandler::getPhonesAsString(string arr[], int nrOf, int priceOfPhone) const
{
    for (int i = 0; i < nrOf; i++)
    {
        arr[i] = this->phone[i]->toString();
    }
}

Upvotes: 1

Views: 44

Answers (1)

R Sahu
R Sahu

Reputation: 206747

The problem is caused by an unwanted ;.

    if (this->phone[i]->getBrand() == name);  // if ends here.

The next block is executed for all items.

    {
        index = i;
        phoneFound = true;
        this->nrOfPhonesInArr--;            
    }

Remove that ; in the if line.

Upvotes: 3

Related Questions