Herezje młodego informatyka- Co zrobiłem źle- QTableWidget

VoidWalker

Witam ponownie

Jestem w trakcie pisania programu na zaliczenie drugiego semestru informatyki. Jak na razie ze wszystkimi moimi bolączkami pomogliście mi po mistrzowsku i za to dziękuje.
No... ale nie pisałbym przecież gdybym nie miał kolejnego problemu, prawda?

Przechodząc to meritum:
Program się wywala kiedy próbuję umieścić w komórce tabeli tekst.
Kod wygląda miej więcej tak:

void MainWindow::on_actionOd_wie_list_triggered()
{
    QFile file("dane.txt");
    if (file.open(QFile::Text | QFile::ReadOnly))
    {
        const QString data(file.readAll());
        const QStringList pieces(data.split(";"));

        for (auto piece : pieces)
        {
            QString imieNazwisko = pieces[1] + " " + pieces[0];
            ui->tableWidget->item(0,PACJENT)->setText(imieNazwisko);

        }
    }
}

Tekst jest pobierany z pliku txt i cięty na interesujące mnie kawałki. Dane które chcę wstawić w pierwszą komórkę wstawiłem wcześniej do jednej zmiennej (tu jest w porządku, do labela wstawia prawidłowo). Problem zaczyna się teraz. Za każdym razem gdy wywołuję całość przyciskiem, cały program się zawiesza, dając taki oto komunikat:

Proces zatrzymany, ponieważ otrzymał on sygnał z systemu operacyjnego.

Nazwa sygnału: SIGSEGV
Znaczenie sygnału: Segmentation fault
*

Chris Kawa

Hej, ten kod zakłada wiele rzeczy, które wcale nie muszą być spełnione.

Czy w tableWidgecie jest wiersz 0? Jak nie to będzie crash.
Czy tableWidget ma wystarczającą ilość kolumn (innymi słowy czy zmienna PACJENT nie wychodzi poza zakres)? Jak nie to crash.
Zakładając, że taka kolumna i wiersz istnieją to czy jest w tej komórce item? Jeśli nie to funkcja item(0,PACJENT) zwraca nullptr i zawołanie na tym setText zakończy się crashem.
Nie sprawdzasz czy na liście pieces rzeczywiście są przynajmniej 2 elementy. Jeśli nie to zawołanie pieces[0] albo pieces[1] zakończy się crashem.

Z innych uwag nie powodujących crasha:

Jak wspomniałem w innym wątku zamiast split lepiej używaj splitRef, unikniesz niepotrzebnych kopii.
W forze wykonujesz dokładnie tą samą operację tyle razy ile jest kawałków w pieces za każdym razem nadpisując dokładnie tą samą komórkę. Chyba nie o to Ci chodziło.
for (auto piece : pieces) to wykonuje wiele niepotrzebnych kopii. Zamiast tego użyj for (const auto& piece : pieces), a na przyszłość, jeśli kontener nie jest const (tu akurat jest), to for (const auto& piece : qAsConst(pieces)).
Skoro dzielisz string po ";" tylko po to żeby go zaraz znów połączyć za pomocą " " to może lepiej po prostu podmienić średnik na spację za pomocą replace?

VoidWalker

@Chris-Kawa

W tableWidget jest na razie tylko jeden wiersz, a że notacja w qt jest Amerykańska, to powinien być oznaczony jako 0 (chyba?)
kolumny są trzy, nazwy zdefiniowane w mainwindow.h (PACJENT jako 0)
cóż... komórka jest pusta. Czy item(0,PACJENT) nie jest w tym wypadku odwołaniem do konkretnej komórki z możliwością przypisania kolejnych poleceń?
Co do sprawdzania czy pieces[0] i peces[1] nie są puste, to fakt, będę musiał zaimplementować takie zabezpieczenie, jednak załóżmy na razie, że wszystko jest załadowane poprawnie

Co do innych uwag

Pamiętam o tym wątku, po prostu jeszcze nie zdążyłem wprowadzić zmian :)
"może lepiej po prostu podmienić średnik na spację za pomocą replace?"; albo mógłbym połączyć te dwa elementy już w momencie zapisywania danych do pliku

Po dłuższej chwili dochodzę do wniosku że muszę jeszcze raz przemyśleć sposób w jaki dane są zapisywane i odczytywane.
Dalej jednak, co w takim razie z tą tabelą? Co wg Ciebie powinienem zmienić jeżeli chodzi o samo wstawianie danych do poszczególnych komórek? Oczywiście, pomijając fakt, że prawdopodobnie większość mojego kodu to jeden wilki bezsens i herezja :P

Chris Kawa

Komórka może być "pusta" w sensie wizualnie pustego prostokąta w dwóch przypadkach - albo nie ma w niej itema albo item jest i ma ustawiony pusty tekst.
Strzelam, że nie wstawiłeś tam nic wcześniej i sypie się ponieważ item(...) zwraca nullptr.
Do wstawiania itemów służy metoda setItem(). Możesz albo wstawić je gdzieś wcześniej w programie i na wciśnięcie przycisku tylko ustawiać tekst (tak jak masz teraz) albo dodać sprawdzenie czy item(...) zwraca nullptr. Jeśli nie to można na nim zawołać setText. Jeśli tak to trzeba najpierw wstawić item za pomocą setItem.

VoidWalker

@Chris-Kawa
Wielkie dzięki Chris!!!
Biorę się do roboty i zobaczymy co z tego wyjdzie :)

VoidWalker

No dobra
setItem działa więc nie powinienem mieć więcej problemów jeżeli chodzi o tabelę.
Mam za to jeszcze dwa pytania odnośnie splitRef

Czy można umieścić jednego splita w drugim? tzn w moim przypadku pierwszy rozdzielałby poszczególnych "pacjentów" w pliku tekstowym, a drugi dzieliłby dane tych pacjentów na potrzebne mi kawałki.
Napotkałem problem ze splitRef.
kod wygląda

const QStringList pieces(data.splitRef("*",QString::SplitBehavior::KeepEmptyParts,Qt::CaseSensitive));

jednak pojawiają się błędy które dotyczą nie tyle splitRef-a co chyba samego QStringList:

C:\Users\B\Desktop\Projekt program\EyeLog\mainwindow.cpp:34: błąd: no matching function for call to 'QStringList::QStringList(QVector<QStringRef>)'
const QStringList pieces(data.splitRef("*",QString::SplitBehavior::KeepEmptyParts,Qt::CaseSensitive));
^

Chris Kawa

splitRef nie zwraca QStringList. O to w nim przecież chodzi żeby nie robić kopii stringów. Typ zwracany to QVector<QStringRef>. Przy okazji możesz pominąć nazwę enuma i skrócić trochę ten zapis:

const QVector<QStringRef> pieces = data.splitRef("*", QString::KeepEmptyParts, Qt::CaseSensitive);

albo prościej

const auto pieces = data.splitRef("*", QString::KeepEmptyParts, Qt::CaseSensitive);

Te parametry, które podajesz mają też takie same wartości domyślne czyli możesz nawet tak i wyjdzie to samo:

const auto pieces = data.splitRef("*");

I taka mikro-optymalizacja jeszcze - skoro napis, po którym dzielisz ma tylko jeden znak to lepiej użyć overloada, który bierze znak a nie string:

const auto pieces = data.splitRef('*');