Skip to content

separate sequence of same numbers from string

An answer to this question on Stack Overflow.

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.

Answer

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;
}