Question

EDIT: To clarify, the object array I am searching is indeed presorted in alphanumeric order by the variable being searched.

I've made a binary search function and nested it in another function. For some reason, every time I use the binary search it fails to find the relevant character array.

Basically, binary search is supposed to search through the return value of getAccountNumber() for each object in the array of objects called credArray. The binary search always returns -1.

The whole program compiles fine, it just doesn't produce the right results. Using the array of objects' methods works elsewhere too, so that is not a problem. I know I made some incredibly simple error but I do not see it. These are the two functions:

//This function uses binary search to search for the relevant account
int AccountDB::searchForAccount(char* searchNumber)
{
    int low = 0;
    int high = accountsAmount - 1;
    int mid;
    //now for the loop, using strcmp because these are C strings
    while (low<=high)
    {
        mid = (low + high) / 2;
        if (strcmp(searchNumber,credArray[mid].getAccountNumber()) == 0)
            return mid;
        if (strcmp(searchNumber,credArray[mid].getAccountNumber()) > 0)
            high = mid -1;
        else
            low = mid + 1;
    }
    return -1;
}
//This function records the transactions and writes the output to an external file
void AccountDB::processTransactions(const char* transactFile)
{
    //set up the input stream from the text file
    ifstream inFile;
    //set up the variables to be read from text file
    char date[6];
    char type;
    char accountnumber[20];
    double amount; 

    //open the file
    inFile.open(transactFile);
    //standard check for file and exit if it doesn't exist
    if(!inFile)
    {
        cout << "Error, input file could not be opened.\n";
        exit(1);
    }
    //Creates a header for listing transactions
    cout << setw(5) << "Date"
         << setw(25) << "Account Number"
         << setw(5) << "Type"
         << setw(8) << "Amount"
         << setw(30) << "New Balance"
         << endl;
         inFile >> date;
         while (inFile)
         {
            inFile >> accountnumber >> type >> amount;
            cout << setw(5) << date
                 << setw(25) << accountnumber
                 << setw(5) << type
                 << setw(8) << amount;
            int relevantAccount = searchForAccount(accountnumber);
            cout << "\nThe searchForAccount returned " << relevantAccount << " by the way\n";
            if (relevantAccount != -1)
            {
                if (type == 'P')
                {
                    credArray[relevantAccount].processPayment(amount);
                    cout << setw(30) << credArray[relevantAccount].getBalance() << endl;
                }
                else
                {
                    bool chargestatus = credArray[relevantAccount].processCharge(amount);
                    if (chargestatus = 1)
                        cout << setw(30) << credArray[relevantAccount].getBalance() << endl;
                    else
                        cout << "Credit limit exceeded" << endl;
                }
            }
            else
                cout << "Invalid account number" << endl;   
            inFile >> date;
        }
        cout << "End of transaction list." << endl;
}
Was it helpful?

Solution

You're right there is a very simple little error. The strcmp() return value comparison is greater than 0 when the first string is greater than the second string. Which means that your comparisons should be flipped.

Here is the fixed code:

int AccountDB::searchForAccount(char* searchNumber)
{
    int low = 0;
    int high = accountsAmount - 1;
    int mid;
    //now for the loop, using strcmp because these are C strings
    while (low<=high)
    {
        mid = (low + high) / 2;
        if (strcmp(searchNumber,credArray[mid].getAccountNumber()) == 0)
            return mid;
        if (strcmp(searchNumber,credArray[mid].getAccountNumber()) < 0) // this is now "<"
            high = mid -1;
        else
            low = mid + 1;
    }
    return -1;
}

Update: Small Optimisation

Consider making this change to make it run a little faster, it's also easier to read.

    while (low<=high)
    {
        mid = (low + high) / 2;
        int val = strcmp(searchNumber,credArray[mid].getAccountNumber());
        if (val == 0)
            return mid;
        if (val < 0)
            high = mid -1;
        else
            low = mid + 1;
    }

OTHER TIPS

You are taking the wrong half each time. (If strcmp is bigger than zero, the search item is bigger, so you need to be looking through the high half (ie changing low).)

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top