Qt - QTableWidget - dodawanie i usuwanie wierszy

na razie z QAbstractItemModel dałem sobie na luz, bo muszę przećwiczyć kila rzeczy... ale mam pytanie odnośnie innego obiektu QTableWidget

Czy w tym kodzie dodawanie i usuwanie wierszy jest wystarczająco dobrze napisane ? Czy dałoby się to ulepszyć lub coś zmienić/poprawić ?

poniżej cały kod

MainWindow.h

#ifndef MAINWINDOW_H
#define MAINWINDOW_H

#include <QMainWindow>
#include <QWidget>
#include <QVBoxLayout>
#include <QHBoxLayout>
#include <QPushButton>
#include <QTableWidget>

class MainWindow : public QMainWindow
{
    Q_OBJECT
public:
    explicit MainWindow(QWidget *parent = nullptr);
    ~MainWindow();

private:
    QWidget *mWidget;
    QVBoxLayout *mLayout;
    QHBoxLayout *pbLayoutTable;
    QHBoxLayout *pbLayoutMain;

    QTableWidget *tableWidg;

    QPushButton *pbOpenFile, *pbSaveFile, *pbQuit, *pbAddRow, *pbRemoveRow;

private slots:
    void myAddRow();
    void myRemRow();

signals:
    void myRow(int);
};

#endif // MAINWINDOW_H

MainWindow.cpp

#include "mainwindow.h"
#include <QDebug>

MainWindow::MainWindow(QWidget *parent) : QMainWindow(parent)
{
    resize(600, 370);
    mWidget = new QWidget(this); //![1]
    mLayout = new QVBoxLayout(mWidget); //![2]

    pbLayoutTable = new QHBoxLayout(); //![3]
    pbLayoutMain = new QHBoxLayout(); //![4]

    pbOpenFile = new QPushButton(mWidget);
    pbOpenFile->setText("Open CSV file");

    pbSaveFile = new QPushButton(mWidget);
    pbSaveFile->setText("Save as CSV file");

    pbQuit = new QPushButton(mWidget);
    pbQuit->setText("Quit");

    pbAddRow = new QPushButton(mWidget);
    pbAddRow->setText("Add row");

    pbRemoveRow = new QPushButton(mWidget);
    pbRemoveRow->setText("Remove row");

    tableWidg = new QTableWidget();

    mWidget->setLayout(mLayout);

    pbLayoutTable->addWidget(pbAddRow);
    pbLayoutTable->addWidget(pbRemoveRow);

    pbLayoutMain->addWidget(pbOpenFile);
    pbLayoutMain->addWidget(pbSaveFile);
    pbLayoutMain->addWidget(pbQuit);

    mLayout->addWidget(tableWidg);
    mLayout->addLayout(pbLayoutTable);
    mLayout->addLayout(pbLayoutMain);

    QObject::connect(pbQuit, &QPushButton::clicked, this, &MainWindow::close);

    QObject::connect(pbAddRow, &QPushButton::clicked, this, &MainWindow::myAddRow);
    QObject::connect(this, &MainWindow::myRow, tableWidg, &QTableWidget::insertRow);

    QObject::connect(pbRemoveRow, &QPushButton::clicked, this, &MainWindow::myRemRow);

    setCentralWidget(mWidget);
}

void MainWindow::myAddRow()
{
    int myRowCnt;
    myRowCnt = tableWidg->rowCount();

    if(myRowCnt == 0){
        emit myRow(myRowCnt);

        for(int i=0; i<5; ++i){
            tableWidg->insertColumn(i);
        }
    }
    else{
        emit myRow(myRowCnt);
    }
}

void MainWindow::myRemRow()
{
    tableWidg->removeRow(tableWidg->currentRow());
}

MainWindow::~MainWindow()
{
    delete pbRemoveRow;
    delete pbAddRow;
    delete pbQuit;
    delete pbSaveFile;
    delete pbOpenFile;
    delete pbLayoutMain; //![4]
    delete pbLayoutTable; //![3]
    delete mLayout; //![2]
    delete mWidget; //![1]
}

main.cpp

#include "mainwindow.h"

#include <QApplication>

int main(int argc, char *argv[])
{
    QApplication a(argc, argv);
    MainWindow w;
    w.show();
    return a.exec();
}

nie sprawdziłem czy rzeczywiście działa dobrze, ale jeśli pytasz ogólnie o kod to kilka uwag napiszę:

• nie musisz usuwać w destruktorze tych wskaźników (nawet jest to niewskazane), ponieważ są one dziećmi okna, więc to okno jest odpowiedzialne za ich usunięcie
• przyciski mają w kontsruktorze parametr od ustawienia tekstu, więc możesz go wykorzystać (zawsze kilka linii kodu mniej)
• sygnał myRow nic mi nie mówi. Z tego co widzę to jest on emitowany przy dodawaniu wiersza, więc nazwa mogłaby jakoś na to wskazywać
• layouty nie muszą być memberami Twojej klasy: wystarczy utworzyćje lokalnie, ponieważ rodzic i tak je trzyma w panięci (i sam usunie), a Ty raczej nie będziesz potrzebować się do nich odwoływać.
• mWidget mogłoby mieć jakąś lepszą nazwę
• w pętli używasz "magic number"

zkubinski napisał(a):

void MainWindow::myAddRow()
{
    int myRowCnt;
    myRowCnt = tableWidg->rowCount();

    if(myRowCnt == 0){
        emit myRow(myRowCnt);

        for(int i=0; i<5; ++i){
            tableWidg->insertColumn(i);
        }
    }
    else{
        emit myRow(myRowCnt);
    }
}

Po co ta pętla for na dodatek wywołująca insertColumn? Na pewno chcesz dodawać 5 kolumn w momęcie dodania wiersza?
Z tego co widzę wystarczy:

void MainWindow::myAddRow()
{
     emit myRow(tableWidg->rowCount());
}

A tak naprawdę i tego też można się pozbyć.

@MarekR22:

Na pewno chcesz dodawać 5 kolumn w momęcie dodania wiersza?

na chwilę obecną tak, muszę się nauczyć wiele elementów, to jest jeden z kroków który wykonuję w celach nauki Qt

ale rozwiązanie które dajesz zaraz przetestuję

@_dominik:

nie musisz usuwać w destruktorze tych wskaźników (nawet jest to niewskazane), ponieważ są one dziećmi okna, więc to okno jest odpowiedzialne za ich usunięcie

ale przecież usuwam w odwrotnej kolejności do tworzenia obiektów - czyli w pierwszej kolejności usuwam obiekty tworzone na samym końcu. Więc to jest źle ?

przyciski mają w kontsruktorze parametr od ustawienia tekstu, więc możesz go wykorzystać (zawsze kilka linii kodu mniej)

no w sumie racja ale to co robię, to robię po to aby zapoznać się z funkcjonalnością i nauczyć się z tego korzystać. Jak będę biegły to będę szedł "na skróty" na tym etapie sobie odradzam.

sygnał myRow nic mi nie mówi. Z tego co widzę to jest on emitowany przy dodawaniu wiersza, więc nazwa mogłaby jakoś na to wskazywać

zgoda, ale nie mogę mieć dwóch tych samych nazw... chyba, że się mylę ?

layouty nie muszą być memberami Twojej klasy: wystarczy utworzyćje lokalnie, ponieważ rodzic i tak je trzyma w panięci (i sam usunie), a Ty raczej nie będziesz potrzebować się do nich odwoływać.

a jak to zrobić ? bo nie bardzo rozumiem ? rzucisz przykład kodu ? - już rozumiem, chodzi o to, żeby utworzyć jako obiekty w konstruktorze

mWidget mogłoby mieć jakąś lepszą nazwę

"m" jak "main" ;) ale nadal mam problem z nadawaniem nazw

w pętli używasz "magic number"

jaki "magic" ? co tam "magicznego" ? :)

zkubinski napisał(a):

@_dominik:

nie musisz usuwać w destruktorze tych wskaźników (nawet jest to niewskazane), ponieważ są one dziećmi okna, więc to okno jest odpowiedzialne za ich usunięcie

ale przecież usuwam w odwrotnej kolejności do tworzenia obiektów - czyli w pierwszej kolejności usuwam obiekty tworzone na samym końcu. Więc to jest źle ?

Qt ma mechanizm drzewa i relacji rodzic dziecko.
Jak rodzić idzie do piachu, to pociąga za sobą wszystkie swoje dzieci (gałęzie drzewa).
Jeśli wszystkie twoje obiekty mają rodzica, to zostaną one usunięte automatycznie. _dominik zachęcał cię do usunięcia zbędnego kodu.

poprawiłem już kod i sloty za dodawanie i usuwanie wierszy wyglądają tak

void MainWindow::myAddRow()
{
    int myRowCnt;
    myRowCnt = tableWidg->rowCount();

    if(myRowCnt == 0){
        emit myRow(myRowCnt);

        for(int i=0; i<5; ++i){
            tableWidg->insertColumn(i);
        }
    }
    else{
        emit myRow(myRowCnt);
    }
}

void MainWindow::myRemRow()
{
    tableWidg->removeRow(tableWidg->currentRow());

    if(tableWidg->rowCount() == 0){
        for(int i=5; i>=0; --i){
            tableWidg->removeColumn(i);
        }
    }
}

chociaż zastanawiam się czy dałoby radę to inaczej zrobić np wykorzystując do tego celu

QTableWidgetItem *currentItem() const

która znajduje się w klasie QTableWidget