0

I am writing a program for a class with the following description:

"Eve was a beautiful Princess, and many suitors came courting her. She decided that the following procedure would decide who she would marry. She would line the men up, and give them a number based on their position (1 for first in line, 2 for second, and so on). Whenever she reached the third person in line, that person would be elimated from the line. Whenever she reached the end of the line, she would continue counting from the beginning. The last person in line would have the honor of marrying her."

The problem is, every time I run the code, I get an outrageously large number as my answer. Testing with cout shows that the program doesn't touch the contents of the while loop. When the while loop is removed, the program crashes with 'vector iterator + offset out of range' and "Standard C++ libraries out of range".

I understand that there is something wrong within the while loop itself that makes the program crash, and I have been working on it for days to try to figure out what. What I do not understand is why the program ignores the while loop in the first place. Any and all advice would be greatly appreciated.

// EveSuitor.cpp : Defines the entry point for the console application.
//

#include "stdafx.h"
#include <vector>
#include <iostream>
using namespace std;

class EveSuitor
{
    vector<int> newList;

public:

    EveSuitor::EveSuitor(vector<int> list)
    {
    vector<int> newList = list;
    }


    void setUpSuitors(vector<int>list);
    int chooseWinner(vector<int>list);

    int suitorCounter;
    int suitorTotal;

};



void EveSuitor::setUpSuitors(vector<int>v)
{
    int numOfSuitors;
    cout<< "Enter in the number of suitors that are there: ";
    cin>> numOfSuitors;

    for (int i = 0; i < numOfSuitors; i++)
    {
        v.push_back(i);
    }

    for (unsigned int j = 0; j < v.size(); j++)
    {
        cout<< v[j] << " ";
    }
}
int EveSuitor::chooseWinner(vector<int>v)
{
    while (v.size() > 1)
    {
        v.erase(v.begin()+suitorTotal);

        for (unsigned int j = 0; j < v.size(); j++)
        {
            cout<< v[j] << " ";
        }   

        suitorTotal = suitorTotal + suitorCounter;

        if (suitorTotal > v.size())
        {
            suitorCounter = suitorTotal - v.size();//This will reset the counter for the suitor, so that it will move on to the next suitor at the beginning of the list.
            suitorTotal = 0;
        }
        else
        {
            suitorCounter = suitorCounter + 3;
            suitorTotal = suitorTotal + 3;
        }
    }
    if (v.size() == 1)
    {
        return v[0];
    }

}

int _tmain(int argc, _TCHAR* argv[])
{

    int suitorTotal = 3;
    int suitorCounter = 3;
    vector<int>listOfSuitors;

    EveSuitor list(listOfSuitors);

    list.setUpSuitors(listOfSuitors);
    int winner = list.chooseWinner(listOfSuitors);

    cout<<"The position you should be at is: " << winner;
    return 0;
}
user3280790
  • 27
  • 6
  • It will only enter the while loop if the size of the list is greater than 1. Are you sure the list is being populated as expected? – charlieparker Feb 06 '14 at 23:48
  • Thanks to Xarn's help, the list is being populated. As for the reason it crashes, that's still a mystery. – user3280790 Feb 06 '14 at 23:59
  • 1
    @user3280790 You should always make sure that function reach an actualy `return` if they do return something. That might very well be the cause of crashes. – s3rius Feb 07 '14 at 00:24
  • @user3280790 Also, `EveSuitor::suitorTotal` is used in the while loop before you write to it. So there's probably a large garbage value in it when the loop runs for the first time, causing `v.erase(v.begin()+suitorTotal)` to go boom. – s3rius Feb 07 '14 at 00:28
  • @serius Thanks to the help of Xarn, hopefully the code will not go boom now... Hopefully. – user3280790 Feb 07 '14 at 01:09

2 Answers2

1

Your list isn't being populated. Your setUpSuitors function has signature of setUpSuitors(vector<int> v), which means that the vector will be passed by value, that is, the function will have access to copy of the vector. If you want to modify the vector inside the function, you will need to pass it by reference, like this: setUpSuitors(vector<int>& v) (note the &).

The reason you get some ludicrous number is that you do not return from chooseWinner, as you take the false branch of the if guarding the return v[0]; statement. (Implementation details cause pseudorandom value to be used instead of return.)


As for the crashes:

The error message tells you that the problem happens when trying to delete from vector by using v.erase(v.begin()+suitorTotal);. There are two problems with that

1) Neither of suitorTotal and suitorCounter are actually initialized inside the class. You could for example pass them in the constructor to fix this, or set them afterwards.

2) Unless the name is quite misleading, v.begin() + suitorTotal will give you iterator equivalent with v.end(), which doesn't work in erase. You need to rethink the logic of selecting which suitor to delete next.

Xarn
  • 3,460
  • 1
  • 21
  • 43
  • Thank you for helping me fix that problem. It now takes in the input just fine. Still need to figure out why it crashes after v.erase(v.begin()+suitorTotal); – user3280790 Feb 06 '14 at 23:58
  • @user3280790 Well, try initializing `suitorTotal` and `suitorCounter` inside the class, not inside the `main`. You could (and probably should), add two more arguments to your constructor, to pass these two inside. – Xarn Feb 07 '14 at 00:05
  • Thank you for all your help so far. It now loops twice, then crashes (when given an input of 6), so more work will be needed, but I think I can handle it... Hopefully. – user3280790 Feb 07 '14 at 01:03
  • @user3280790 To be honest, the logic for deleting suitors looks odd, you should probably rethink it. (Note: `v.erase()` has to be given valid iterator to an item inside the vector. The error you gave is caused by you giving invalid iterator and your STL implementation having checked iterators.) – Xarn Feb 07 '14 at 01:07
  • Maybe it's just me, but I am having a hard time thinking up a different way to delete suitors (I am not saying you have to figure one out either- it's my project, I should not have you do all the work). I will keep looking at it, and testing to figure out something else. Thank you for the information about the iterators, though. – user3280790 Feb 07 '14 at 01:16
  • @user3280790 Oh there is no problem with the way you delete them (`v.erase()`), just the logic of picking which one is weird. – Xarn Feb 07 '14 at 10:13
  • Fair enough. I will talk with my teacher about it. Thanks again fro all the help. – user3280790 Feb 07 '14 at 17:18
0

Your list isn't being populated correctly. Read up on passing by reference vs value.

This question has some good answers:

Pass by Reference / Value in C++

Community
  • 1
  • 1
Force444
  • 3,321
  • 9
  • 39
  • 77