separate sequence of same numbers from string

Question

I'm trying to do a little task which asks to convert numbers to letters of phone keypad, for example if input is 222 it means phone button "2" ( http://upload.wikimedia.org/wikipedia/commons/7/7d/Telephone-keypad.png ) is pushed 3 times and output should be "C" and etc. So first thing what i should do is to separate all the sequences, for example 22255-444 into 222 , 55 , - , 444 and then i think figure everything out, but now problem is that my function can't read last sequence

#include <iostream>
#include <fstream>
using namespace std;
//-------------------------------------------------------------------------

void encode(string text, string &result, int &i)
{
 char keyboard[10][4] = {
    {' ',' ',' ',' '},
    {'.','?','!','.'},
    {'a','b','c','a'},
    {'d','e','f','d'},
    {'g','h','i','g'},
    {'j','k','l','j'},
    {'m','n','o','m'},
    {'p','r','q','s'},
    {'t','u','v','t'},
    {'w','x','y','z'}
  };

  int j;
  for(j = i; j<text.size();j++)
  {
    if(text[i] != text[j] || j == text.size())
    {
        result = text.substr(i, j-i);
        i = j-1;
        break;
    }

  }
  cout << result << endl;

}



int main()
{
  ifstream fd("sms.in");
  string text;
  string result;
  getline(fd, text);
  for(int i = 0; i<text.size();i++)
  {
    encode(text, result, i);
  }
  return 0;
}

as a test now im using this input : 5552-22-27777 , output should be 555 2 - 22 - 2 7777, but for me its 555 2 - 22 - 2 2 2 2 2.

Well i'd say throw it at the debugger and see what part is not doing what it should. — RedX
– RedX, Commented Dec 12, 2012 at 9:21
Bunch of text disappeared from my answer, but it's back now. — Richard
– Richard, Commented Dec 12, 2012 at 9:42

interjay · Accepted Answer · 2012-12-12 09:24:52Z

2

In this if statement:

if(text[i] != text[j] || j == text.size())

The second condition (j == text.size()) will never be true because the loop will terminate before that. So when you reach the end of the string, the values of result and i will not be updated correctly.

What you can do is remove the termination condition from the loop (it's not necessary to have one because you will break out of the loop anyway). And you will need to reverse the order of the conditions in the if so that you don't read past the end of the string:

for(j = i; ;j++)
{
    if (j == text.size() || text[i] != text[j])
    ...

answered Dec 12, 2012 at 9:24

interjay

111k22 gold badges278 silver badges264 bronze badges

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

Comments

Richard · Accepted Answer · 2012-12-12 09:39:12Z

I'd pull that last bit out of the for loop, it doesn't really belong in there because it's a special case. I've also eliminated some other things that seemed unnecessary and fixed the inside of the loop up. See if this all makes sense to you.

I pulled the keymap declaration out because it was not used in your example, and we value what are known as "minimal working examples" here as a way to isolate and discuss problems cogently.

Note that I've tried to move all the encoding work into a single function and out of the main() function. This frees up main to handle high-level parts of the program. On the lines I've marked "Decode here" you could call to yet another function that translates the string of numbers into a character, if that's what you're going for.

I've marked the special case and explicitly moved it out of the for-loop so that everything within the for-loop can be handled the same. This is better than trying to make the for-loop do everything.

I didn't agree with the logic that i=j-1, so I changed that (I did test this).

I've also switched to read input directly from the command line so that it's quicker to test.

I added the #include <string> directive at the beginning so that the program would compile.

Putting all the encoding work inside a single function simplified that functions declaration to a single argument.

I switched the brackets to conform to my own understanding of the One True Style.

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

void encode(string text) {
  int j=0,i=0; //Declare j here so we can use in special case
  for(j=0; j<text.size(); j++){
    if(text[j] != text[i]){
      cout << text.substr(i, j-i) << endl; //Decode here
      i = j;
    }
  }
  cout << text.substr(i,j-1) << endl; //Decode here. Special case
}

int main(){
  string text;

  getline(cin, text);
  encode(text, result);

  return 0;
}

Collectives™ on Stack Overflow

separate sequence of same numbers from string

2 Answers 2

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related