Usuwanie spacji obok siebie

Question

Czesc pisze zadanie z C++ 0x

i musze podmienic oraz usunac spacje ktore wystepuja obok siebie. Podmienianie działa ale nie usuwanie spacji znaczy usuwa ale tylko czesc tych spacji 

#include <iostream>
#include <string>

using namespace std;

string konwertuj( string & sTekst )
{
    string sWynik;

    for(int i=0; i<=sTekst.size(); i++) {

        if(sTekst[i] == '<') {

            sTekst.erase(i, 1);
            sTekst.insert(i, "[");
        }
        if(sTekst[i] == '>') {

            sTekst.erase(i, 1);
            sTekst.insert(i, "]");
        }
        if(sTekst[i] == ' ' && sTekst[i+1] == ' ') {
            sTekst.erase(i, 1);
        }
    }
    sWynik = sTekst;
    return sWynik;
}
int main()
{
    string tekst = "<b>to jest </b> testowy       napis     <b>:)";
    cout << tekst<<endl;
    cout << konwertuj( tekst)<<endl;

    return 0;
}

 

Answer 1

Ten kod jest strasznie przekombinowany.

1. std::string w C++ nie jest immutable, więc zamiast:

   sTekst.erase(i, 1);
   sTekst.insert(i, "[");

   które tak na marginesie sprawia, że cały string(lub tylko czesc z prawej strony) jest przepisywany dwa razy :D zrob po prostu tak:

   sTekst[i]='[';

2. 	sWynik = sTekst;
   	return sWynik;

   możesz po prostu:

   return sTekst;

3. Po co ten przedrostek przed nazwą zmiennych. Czyżby od typu? Takie praktyki to przestarzałość. Dzisiaj mamy IDE i wystarczy najechać na nazwe zmiennej zeby sprawdzic jej typ.

4. for (int i = 0; i <= sTekst.size(); i++) {
   ...
   sTekst[i + 1]

   Wychodzisz poza zakres tablicy. i maksymalnie jest rowne size, wiec odwolujesz sie do sTekst[size+1]. O 2 indeksy za stringiem. Technicznie rzecz biorąc o 1, bo s[s.size()] jest zawsze znakiem null '\n'.

A teraz do rzeczy:

		if (sTekst[i] == ' ' && sTekst[i + 1] == ' ') {
			sTekst.erase(i, 1);
		}

usuwasz jedną spacje. Usuwanie ze stringa w petli nie jest najlepszym pomyslem, bo po pierwsze trzeba modyfikować iterator i ręcznie, co jest nieintuicyjne i tak jak wspomnialem przy kazdym usunieciu elementy po prawej są przepisywane o 1 miejsce w lewo, więc lepiej już wpisywać do nowego stringa elementy, które zostają zamiast usuwać ze starego w miejscu.

Normalnie użyłbym idiomu remove-erase, ktory wydajnie usuwa wiele elementow ze srodka kontenera bez ciaglego przepisywania elementow, ale tutaj chodzi o usuniecie roznych podstringow - zlozonych z roznej ilosci spacji obok siebie. Co od razu narzuca regexy:

regex_replace(tekst, regex{ "\\s{2,}" }, " ");

EDIT: jesli nie znasz regexow i na razie nie chcesz poznwac to mozna to zrobic np tak:

string konwertuj(string & sTekst)
{
	string sWynik;
	bool wasSpace = false;
	for (char c : sTekst) {
		if (c != ' ') {
			wasSpace = false;
			sWynik += c;
		}
		else if (!wasSpace) {
			sWynik += c;
			wasSpace = true;
		}
	}
	return sWynik;
}

lub tak:

string konwertuj(string & sTekst)
{
    string sWynik;
    if (sTekst.size() > 0) {
        sWynik += sTekst[0];
    }
    for (size_t i = 1; i < sTekst.size(); i++)
    {
        if (sTekst[i] != ' ' || sTekst[i - 1] != ' ') {
            sWynik+=sTekst[i];
        }
    }
    return sWynik;
}

 

Comments

Do usuwania nadmiaru spacji można użyć std::unique.

Answer 2

Cześć,

Fajnie napisany kod.

Brakuje jedynie jednej linii: i--; w warunku, który usuwa wielokrotne spacje.

if(sTekst[i] == ' ' && sTekst[i+1] == ' ') {
            sTekst.erase(i, 1);
            i--;
        }

Po usunięciu znaku z Tekstu trzeba zmniejszyć iterator pętli, żeby kolejny raz poprawnie przeszukać tekst.

pozdrawiam

Comments

Masz tu UB. Czytasz indeks poza zakresem string'a.

---

Możesz trochę jaśniej ? Czy masz na myśli przypadek gdy pierwsze dwa znaki to spacje?

Można to rozwiązać:

if(sTekst[i] == ' ' && sTekst[i+1] == ' ') {
            sTekst.erase(i, 1);
            if(i) i--;
        }

 

---

Co się stanie jeśli indeks i będzie ostatnim znakiem w string'u? Dostęp do i + 1 nastąpi już "za ostatnim znakiem" string'a. Przez to zachowanie będzie niezdefiniowane.

Poza tym to...

for(int i = 0; i <= sTekst.size(); i++)

.. także jest błędne. Nierówność powinna być "ostra".