Segmentation fault in vector

Question

This is a code that I wrote for a Topcoder SRM. However there is a segmentation fault here that I cannot figure out. Can you help me figure it out?

The problem is -

This task is about the scoring in the first phase of the die-game Yahtzee, where five dice are used. The score is determined by the values on the upward die faces after a roll. The player gets to choose a value, and all dice that show the chosen value are considered active. The score is simply the sum of values on active dice. Say, for instance, that a player ends up with the die faces showing 2, 2, 3, 5 and 4. Choosing the value two makes the dice showing 2 active and yields a score of 2 + 2 = 4, while choosing 5 makes the one die showing 5 active, yielding a score of 5. Your method will take as input a vector toss, where each element represents the upward face of a die, and return the maximum possible score with these values.

Constraints -

toss will contain five elements exactly. Each element will be between 1 and 6

#include <iostream>
#include <vector>

using namespace std;

class YahtzeeScore
{
  public:
  int maxPoints(vector<int> toss);
};

int YahtzeeScore::maxPoints(vector <int> toss)
{ 
  vector<int> ret;
  vector <int>::iterator v = toss.begin();
  vector<int>::iterator w = toss.begin();
  for (;v != toss.end(); v++)
  {
    int s=0;
    for (;w != toss.end(); w++)
    {
      if ( w!=v && *w==*v) s++;
    }
     ret.push_back(s);
  }

  vector<int>::iterator it = ret.begin();
  for (; it!=ret.end(); it++)
  {
    if (*v>*(v+1))
    {
      int temp = *v;
      *v = *(v+1);
      *(v+1)=temp;
    }
  } 
  return ret[4];
}


int main()
{
  YahtzeeScore ob;
  vector<int> something;
  vector<int>::iterator it = something.begin();
  while (it != something.end())
  {
    int a;
    cin >> a;
    something.push_back(a);
  }
  cout << ob.YahtzeeScore::maxPoints(something);
}

You don't re-initialize w = toss.begin(); in every for (;v != toss.end(); v++) so the w iteration works properly only for the forst v loop. — CiaPan
– CiaPan, Commented Apr 15, 2014 at 4:56
In main(), you are doing push_back into the vector you are iterating through. That invalidates the iterator. (Also, what is the point of the iterator there? There vector starts empty.) — jogojapan
– jogojapan, Commented Apr 15, 2014 at 4:57

The Dark · Accepted Answer · 2014-04-15 04:57:17Z

1

Note: You should run it through a debugger.

This bit of code in main():

vector<int>::iterator it = something.begin();
while (it != something.end())

it will always be something.end() as something is empty. This means it never reads any input, which means you pass a empty vector to maxPoints. Neither of maxPoints loops will run as toss is empty and ret is empty. so ret[4] will cause a segfault.

As you know you want 5 inputs, just use something like:

for (int i = 0; i < 5; i++)
{
    int a;
    cin >> a;
    something.push_back(a);
}

answered Apr 15, 2014 at 4:57

The Dark

8,5041 gold badge18 silver badges19 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

M.M · Accepted Answer · 2014-04-15 04:56:59Z

1

On the line if (*v>*(v+1)), v is pointing to the end so you can't dereference it.

I guess you meant it rather than v, however that still needs work as *(it+1) will be invalid if it is pointing to one-before-the-end.

Also you should checkthe size of ret before just doing ret[4]. Or do ret.at(4) so at least you get an exception instead of a segfault.

answered Apr 15, 2014 at 4:56

M.M

143k26 gold badges223 silver badges400 bronze badges

Comments

R Sahu · Accepted Answer · 2014-04-15 04:58:19Z

0

The source of the problem is in this block:

while (it != something.end())
{
  int a;
  cin >> a;
  something.push_back(a);
}

First time the check is made, it == something.end() since something is empty.

You pass the empty object to ob.YahtzeeScore::maxPoints(), which tries to get the 5-th item from an empty vector (return ret[4]), which causes the segmentation violation.

answered Apr 15, 2014 at 4:58

R Sahu

207k15 gold badges168 silver badges291 bronze badges

Comments

CiaPan · Accepted Answer · 2014-04-15 04:58:54Z

0

Do a simple loop for( int face=1; face<6; face++) and scan 6 times the five faces for those equal face. Sum them up and store the maximum sum found. Done.

answered Apr 15, 2014 at 4:58

CiaPan

9,5802 gold badges23 silver badges38 bronze badges

Comments

Sergey Prokhorov · Accepted Answer · 2014-04-15 05:06:23Z

0

Vector something is not intialized, this caused segfault at return ret[4].

vector<int> something;
vector<int>::iterator it = something.begin();
while (it != something.end())

This loop never run because vector something is empty and it points to something.end(), and loop endup with 0 elements in something. Than you call maxPoints considering something contains at least 5 elements. Since something contains nothing, return ret[4]; is incorrect and you got segfaul. Suggestion: - there is clear precondition for maxPoints: toss.size() == 5. Use assert - I would suggest to use maxPoints(const vector<int> & toss);, this will eliminate unnecessary copy.

answered Apr 15, 2014 at 5:06

Sergey Prokhorov

1353 bronze badges

Collectives™ on Stack Overflow

Segmentation fault in vector

5 Answers 5

Comments

Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

Comments

Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related