Интересует мнение по поводу кода, *не знал куда поместить Опции

[RazrFalcon]

Просто интересно, ради повышения опыта.
Хотелось бы услышать все возможные замечания. Может что то делаю совсем не так, как заведено и тд.

Программка довольно простая, банальная смена обоев для Gnome. Просто захотелось довести хоть одну прогу до ума.

Собственно код:

.pro

QT       += core gui

TARGET = wall
TEMPLATE = app

SOURCES += main.cpp\
        wallwindow.cpp

HEADERS  += wallwindow.h

CONFIG(debug, debug|release){
  DEFINES += MY_DEBUG
}

main.cpp

#include <QApplication>
#include "wallwindow.h"

int main(int argc, char *argv[])
{
    QApplication a(argc, argv);
    WallWindow w;
    #ifdef MY_DEBUG
        w.show();
    #endif
    QApplication::setQuitOnLastWindowClosed(false);
    return a.exec();
}

wallwindow.cpp

#include "wallwindow.h"

//! TODO
//only one running program
//create *.deb or autodeb script
//previous
//on add file - wrong width of second colum
//auto update selected folder
//delete direct from wall by using context menu

int switch_time=60, row_num=0, number;
bool ispause=false;

WallWindow::WallWindow()
{
    createLayout();

    settings = new QSettings(QSettings::IniFormat, QSettings::UserScope,
                             "wallpapers-changer", "wallchanger");

    switchTimer = new QTimer(this);
    connect(switchTimer,SIGNAL(timeout()),this,SLOT(setImage()));

    connect(closeButton,SIGNAL(clicked()),this,SLOT(quitButton_clicked()));
    connect(startButton,SIGNAL(clicked()),this,SLOT(startButton_clicked()));
    connect(clearButton,SIGNAL(clicked()),this,SLOT(clearButton_clicked()));
    connect(addButton  ,SIGNAL(clicked()),this,SLOT(addButton_clicked()));

    connect(onlynames_checkBox,SIGNAL(stateChanged(int)),this,
           SLOT(on_onlynames_checkBox_stateChanged(int)));

    connect(timeBox,SIGNAL(valueChanged(int)),this,
           SLOT(on_timeBox_valueChanged(int)));

    connect(pos_comboBox,SIGNAL(currentIndexChanged(QString)),this,
           SLOT(on_pos_comboBox_currentIndexChanged(QString)));

    connect(order_comboBox,SIGNAL(currentIndexChanged(QString)),this,
              SLOT(order_comboBox_currentIndexChanged(QString)));

    connect(table,SIGNAL(cellClicked(int,int)),this,
           SLOT(on_table_cellClicked(int,int)));

    connect(table,SIGNAL(cellDoubleClicked(int,int)),this,
           SLOT(on_table_cellDoubleClicked(int,int)));

    //Tray menu
    QAction* restoreAction = new QAction(tr("&Restore"), this);
    connect( restoreAction, SIGNAL(triggered()), this, SLOT(showNormal()));

    QAction* quitAction = new QAction(tr("&Quit"), this);
    connect( quitAction, SIGNAL(triggered()), qApp, SLOT(quit()));

    QMenu* trayIconMenu = new QMenu(this);
    trayIconMenu->addAction(restoreAction);
    trayIconMenu->addAction(quitAction);

    //Set tray icon
    trayIcon = new QSystemTrayIcon(this);
    trayIcon->setContextMenu(trayIconMenu);
    trayicon = new QIcon("/usr/share/icons/hicolor/48x48/apps/wallpaper-changer.png");
    trayIcon->setIcon(*trayicon);
    trayIcon->show();

    connect(trayIcon, SIGNAL(activated(QSystemTrayIcon::ActivationReason)),
    this, SLOT(iconActivated(QSystemTrayIcon::ActivationReason)));

    delIcon = new QIcon("/usr/share/icons/gnome/48x48/actions/edit-delete.png");

    if (settings->value("settings/show_only_names")==true)
        onlynames_checkBox->setCheckState(Qt::Checked);
    timeBox->setValue(settings->value("settings/scroll_time", 10).toInt());
    scroll_label->setText(settings->value("settings/in", "Scroll time(minutes):").toString());
    pos_comboBox->setCurrentIndex(settings->value("settings/img_position", 0).toInt());
    order_comboBox->setCurrentIndex(settings->value("settings/img_order",0).toInt());

    if (scroll_label->text().contains("hour"))
    {
        timeBox->setMaximum(24);
        timeBox->setMinimum(0);
    }

    setWindowTitle(tr("Wallpapers Changer 0.1"));
    setWindowIcon(*trayicon);

    allFiles=load_and_show();
    rand_numbers=createRandList();
    if (allFiles.count()!=0) startButton_clicked();
}

void WallWindow::createLayout()
{
    imgpos_label = new QLabel("Image position:");
    countlabel = new QLabel("Image count:");
    scroll_label = new QLabel("Scroll time(minutes):");
    scroll_label->setSizePolicy(QSizePolicy::Fixed,QSizePolicy::Fixed);

    startButton = new QPushButton(tr("Start"));
    closeButton = new QPushButton(tr("Quit"));
    clearButton = new QPushButton(tr("Clear"));
    addButton = new QPushButton(tr("Add"));

    //Table configure
    table = new QTableWidget;
    table->setVerticalScrollBarPolicy(Qt::ScrollBarAlwaysOn);
    table->setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOn);
    table->setAutoScroll(false);
    table->setEditTriggers(QAbstractItemView::NoEditTriggers);
    table->setSelectionMode(QAbstractItemView::NoSelection);
    table->setHorizontalScrollMode(QAbstractItemView::ScrollPerPixel);
    table->setShowGrid(false);
    table->horizontalHeader()->setVisible(false);
    table->verticalHeader()->setVisible(false);

    QStringList posList, imgOrderList;
    posList<<"None"<<"Wallpaper"<<"Centered"<<"Scaled"<<"Stretched"<<"Zoom"<<"Spanned";
    imgOrderList<<"Linear"<<"Shuffle";

    timeBox = new QSpinBox;
    pos_comboBox = new QComboBox;
    pos_comboBox->addItems(posList);
    onlynames_checkBox = new QCheckBox(tr("Only names"));
    img_order_label = new QLabel(tr("Order of showing image:"));
    order_comboBox = new QComboBox;
    order_comboBox->addItems(imgOrderList);

    QGridLayout *layout = new QGridLayout;

    layout->addWidget(scroll_label,0,0);
    layout->addWidget(timeBox,0,6,Qt::AlignRight);

    layout->addWidget(countlabel,1,0);
    layout->addWidget(onlynames_checkBox,1,6,Qt::AlignRight);

    layout->addWidget(table,2,0,1,0);

    layout->addWidget(imgpos_label,3,0);
    layout->addWidget(pos_comboBox,3,6,Qt::AlignRight);

    layout->addWidget(img_order_label,4,0,1,0);
    layout->addWidget(order_comboBox,4,6,Qt::AlignRight);

    layout->addWidget(clearButton,5,0);
    layout->addWidget(addButton,5,4);
    layout->addWidget(startButton,5,5);
    layout->addWidget(closeButton,5,6);

    setMinimumHeight(300);
    resize(400,500);
    setLayout(layout);
}

void WallWindow::tableFill()
{
    table->setRowCount(allFiles.count());
    table->setColumnCount(2);

    for(int i=0; i<allFiles.count(); i++)
    {
        QTableWidgetItem* iconItem = new QTableWidgetItem();
        iconItem->setIcon(*delIcon);
        table->setItem(i, 0, iconItem);

        QTableWidgetItem* textItem = new QTableWidgetItem();
        if (onlynames_checkBox->isChecked())
            textItem->setText(QString(allFiles.at(i).fileName()));
        else
            textItem->setText(QString(allFiles.at(i).absoluteFilePath()));
        table->setItem(i, 1, textItem);
    }
    countlabel->setText("Images count: "+QString::number(allFiles.count()));
    table->resizeColumnsToContents();
}


QFileInfoList WallWindow::load_and_show()
{
    QStringList files = settings->value("settings/file_list").toStringList();
    for (int i=0; i<files.count(); i++)
    {
        QFileInfo info(files[i]);
        allFiles.append(info);
    }
    tableFill();
    return allFiles;
}

void WallWindow::startButton_clicked()
{
    if (startButton->text()=="Stop")
    {
        switchTimer->stop();
        qDebug()<<"Scrolling stopped";
        startButton->setText("Start");
    }
    else
    {
        if (scroll_label->text().contains("minutes")) switch_time=timeBox->text().toInt()*60*1000;
        else switch_time=timeBox->text().toInt()*60*60*1000;
        //switch_time=1000; //for debug
        switchTimer->start(switch_time);
        if (switchTimer->isActive()) qDebug()<<"Scrolling started";
        startButton->setText("Stop");
        setImage();
    }

    configSave();

    #ifdef MY_DEBUG
        qDebug().nospace()<<"Switch time: "<<switch_time/(1000*60)<<"min.";
    #endif
}

QList<int> WallWindow::createRandList()
{
    QList<int> temp1, temp2;
    int rand_num;
    qsrand(QDateTime::currentDateTime().toTime_t());

    for (int i=0; i<allFiles.count(); ++i) temp1<<i;
    linear_list=temp1;

    for (int i=0; i<allFiles.count(); ++i)
    {
        rand_num=qrand()%temp1.count();
        temp2<<temp1.at(rand_num);
        temp1.removeAt(rand_num);
    }
    return temp2;
}

int WallWindow::setImage()
{
    //int number;
    if (row_num!=0)
    {
        number=row_num;
        order_comboBox_currentIndexChanged("Linear");
    }
    else number=rand_numbers.first();

    if (order_comboBox->currentText()=="Linear" && row_num==0)
    {
        number=linear_list.first();
        qDebug()<<"Set image:"<<allFiles.at(number).absoluteFilePath();
        processStart("/desktop/gnome/background/picture_filename",
                     allFiles.at(number).absoluteFilePath());
        linear_list.removeFirst();
    }
    else
    {
        qDebug()<<"Set image:"<<allFiles.at(number).absoluteFilePath();
        processStart("/desktop/gnome/background/picture_filename",
                     allFiles.at(number).absoluteFilePath());
        if (row_num==0) rand_numbers.removeFirst();
    }

    table->scrollToItem(table->item(number,1),QAbstractItemView::PositionAtTop);
    table->horizontalScrollBar()->setValue(0);
    trayIcon->setToolTip("Now: "+allFiles.at(number).fileName());
    if (rand_numbers.count()==0) rand_numbers=createRandList();
    if (linear_list.count()==0) createRandList();
    row_num=0;

    #ifdef MY_DEBUG
        qDebug()<<"Image number:"<<number;

        QDateTime dt = QDateTime();
        qDebug()<<dt.currentDateTime();
    #endif

    return 0;
}

void WallWindow::configSave()
{
    if (onlynames_checkBox->isChecked()) settings->setValue("settings/show_only_names", true);
    else settings->setValue("settings/show_only_names", false);

    settings->setValue("settings/scroll_time", timeBox->value());

    if (scroll_label->text().contains("minutes")) settings->setValue("settings/in", "Scroll time(minutes):");
    else settings->setValue("settings/in", "Scroll time(hour):");

    settings->setValue("settings/img_position", pos_comboBox->currentIndex());

    settings->setValue("settings/img_order", order_comboBox->currentIndex());

    QStringList temp;
    for (int i=0; i<allFiles.count(); i++) temp.append(allFiles.at(i).absoluteFilePath());
    settings->setValue("settings/file_list", temp);
}

void WallWindow::on_onlynames_checkBox_stateChanged(int x)
{
    for(int i=0; i<allFiles.count(); i++)
    {
        QTableWidgetItem* Item = new QTableWidgetItem();
        if (x==2) Item->setText(QString(allFiles.at(i).fileName()));
        if (x==0) Item->setText(QString(allFiles.at(i).absoluteFilePath()));
        table->setItem(i, 1, Item);
    }
}

void WallWindow::on_timeBox_valueChanged(int time)
{
    if (time>59 && scroll_label->text().contains("minutes"))
    {
        timeBox->setValue(1);
        timeBox->setMaximum(24);
        timeBox->setMinimum(0);
        scroll_label->setText("Scroll time(hour):");
    }
    if (time==0)
    {
        timeBox->setMaximum(60);
        timeBox->setMinimum(1);
        timeBox->setValue(59);
        scroll_label->setText("Scroll time(minutes):");
    }
}

void WallWindow::on_table_cellClicked(int row, int column)
{
    if (column==0)
    {
        table->removeRow(row);
        allFiles.removeAt(row);
        configSave();
    }
}

void WallWindow::on_table_cellDoubleClicked(int row, int column)
{
    row_num=row;
    if (column==1) setImage();
}

void WallWindow::on_pos_comboBox_currentIndexChanged(QString name)
{
    processStart("/desktop/gnome/background/picture_options",name.toLower());
}

void WallWindow::order_comboBox_currentIndexChanged(QString name)
{
    createRandList();
    if (name=="Linear")
        for (int i=linear_list.first(); i<number+1; i++) linear_list.removeFirst();
}

void WallWindow::iconActivated(QSystemTrayIcon::ActivationReason reason)
{
    if (reason==3 || reason==2)
    {
        qDebug()<<"Fast swich";
        setImage();
    }
    if (reason==4)
    {
        if (ispause==true)
        {
            startButton_clicked();
            qDebug()<<"Scrolling started";
            ispause=false;
        }
        else
        {
            startButton->setText("Start");
            switchTimer->stop();
            ispause=true;
            trayIcon->setToolTip("Scrolling stopped");
            qDebug()<<"Scrolling stopped";
        }
    }
}

void WallWindow::addButton_clicked()
{
    QStringList added_files = QFileDialog::getOpenFileNames(this,
                                           "Open Image(s)",
                                           QDir::homePath (),
                                           "Image Files (*.png *.jpg *.bmp *.tiff *.tif)");

    for (int i=0; i<added_files.count(); i++)
    {
        QFileInfo info(added_files[i]);
        allFiles.append(info);
    }

    tableFill();
    rand_numbers=createRandList();
    configSave();
}

void WallWindow::clearButton_clicked()
{
    allFiles.clear();
    table->clear();
    table->setRowCount(0);
    table->setColumnCount(0);
    configSave();
}

void WallWindow::closeEvent(QCloseEvent * event)
{
    hide();
    event->ignore();
    configSave();
}

void WallWindow::quitButton_clicked()
{
    configSave();
    exit(0);
}

void WallWindow::processStart(QString path, QString arg)
{
    QProcess myProcess;
    QString program = "gconftool-2";
    QStringList arguments;
    arguments<<"--type"<<"string"<<"--set"<<path<<arg;
    myProcess.start(program, arguments);
    myProcess.waitForFinished();
}

wallwindow.h

#ifndef WALLWINDOW_H
#define WALLWINDOW_H

#include <QDialog>
#include <QSystemTrayIcon>
#include <QDir>
#include <QSettings>
#include <QtGui>
#include <QApplication>
#include <QFileDialog>
#include <QtDebug>
#include <QFile>
#include <QProcess>
#include <QSystemTrayIcon>
#include <QMenu>
#include <QAction>
#include <QTimer>

QT_BEGIN_NAMESPACE
class QGroupBox;
class QLabel;
class QTableWidget;
class QSpinBox;
class QComboBox;
class QCheckBox;
class QPushButton;
QT_END_NAMESPACE

class WallWindow : public QDialog
{
    Q_OBJECT

public:
    WallWindow();
    QFileInfoList load_and_show ();

private:
    QFileInfoList allFiles;
    QFileInfo now;
    QStringList added_files;
    QList<int> rand_numbers, linear_list;

    QGroupBox *mainGroupBox;
    QLabel *countlabel;
    QLabel *imgpos_label;
    QLabel *scroll_label;
    QLabel *time_label;
    QLabel *img_order_label;
    QTableWidget *table;
    QSpinBox *timeBox;
    QComboBox *pos_comboBox;
    QComboBox *order_comboBox;
    QCheckBox *onlynames_checkBox;
    QPushButton *startButton;
    QPushButton *closeButton;
    QPushButton *clearButton;
    QPushButton *addButton;
    QTimer* switchTimer;
    QSystemTrayIcon* trayIcon;
    QSettings *settings;
    QIcon *trayicon, *delIcon;

private slots:
    QList<int> createRandList();
    int  setImage();
    void startButton_clicked();
    void quitButton_clicked();
    void iconActivated(QSystemTrayIcon::ActivationReason reason);
    void configSave();
    void tableFill();
    void processStart(QString path, QString arg);
    void on_onlynames_checkBox_stateChanged(int );
    void on_timeBox_valueChanged(int );
    void addButton_clicked();
    void on_table_cellDoubleClicked(int row, int column);
    void on_table_cellClicked(int row, int column);
    void clearButton_clicked();
    void on_pos_comboBox_currentIndexChanged(QString name);
    void order_comboBox_currentIndexChanged(QString name);
    void createLayout();

protected:
    void closeEvent(QCloseEvent *event);
};

#endif // WALLWINDOW_H

Прикрепленные файлы

 wall.zip ( 9,07 килобайт ) Кол-во скачиваний: 126

 

[Алексей1153]

#include <QDialog>
#include <QSystemTrayIcon>
#include <QDir>
#include <QSettings>
#include <QtGui>
#include <QApplication>
#include <QFileDialog>
#include <QtDebug>
#include <QFile>
#include <QProcess>
#include <QSystemTrayIcon>
#include <QMenu>
#include <QAction>
#include <QTimer>

большую часть этих заголовков можно перенести в wallwindow.cpp

код не смотрел, немного некогда, но вот сразу
1)

int switch_time=60, row_num=0,number;
bool ispause=false;

зачем переменные глобальные ? И number не инициализирован

2) нет комментариев. Жирнющий минус

Сообщение отредактировал Алексей1153 - 19.3.2011, 10:57

[Litkevich Yuriy]

WallWindow - наследник QDialog, почему в конструкторе класса не инициализируешь базовый класс? Ты точно понимаешь что происходит?

[RazrFalcon]

большую часть этих заголовков можно перенести в wallwindow.cpp

Зачем? Точнее почему, так тоже ведь работает.

код не смотрел, немного некогда, но вот сразу
1)

int switch_time=60, row_num=0,number;
bool ispause=false;

зачем переменные глобальные?
И number не инициализирован

Глобальные, так как используются не только в одной функции.
Надо будет глянуть.

>> И number не инициализирован
Нужно что то в духе number=0;?

2) нет комментариев. Жирнющий минус

Ну это да, надо на исправить.

>>Ты точно понимаешь что происходит?
По моему нет
А в чем проблема? Вроде работает.

Сообщение отредактировал RazrFalcon - 19.3.2011, 14:27

[Алексей1153]

Зачем? Точнее почему, так тоже ведь работает.

работает. Но, гляжу по содержимому, их присутствие в том файле необязательно, достаточно форвардов. А это значит, что заголовки можно смело перенести в реализацию. Чем меньше левых заголовочников в H файле, тем быстрее компиляция большого проекта, да и глобальная область видимости меньше "замусоривается" левыми определениями (а это частенько помогает избежать запутанных зависимостей между вложенными заголовочниками - когда это действительно бывает необходимо)

Глобальные, так как используются не только в одной функции.

У тебя же класс Сделай членом класса

Нужно что то в духе number=0;?

лучше так (не надо лепить в одну строчку)

int switch_time=60;
int row_num=0;
int number=0;

[RazrFalcon]

>>Но, гляжу по содержимому, их присутствие в том файле необязательно, достаточно форвардов.
ээ... это какие именно? Что нужно оставить в хедере, а что перенести в срр?

>>Сделай членом класса.
Просто перенести в "class WallWindow : public QDialog"?

Сообщение отредактировал RazrFalcon - 19.3.2011, 16:41

[Алексей1153]

RazrFalcon, все, кроме
#include <QDialog>

перенеси, а потом компилятор подскажет (но если от класса используется только ссылка или указатель - достаточно только форвард добавить)

Просто перенести в "class WallWindow : public QDialog"?

ага, туда ) Только инициализацию в конструкторе делать

[RazrFalcon]

Сделал.
В хедере остались только:

#include <QDialog>
#include <QFileDialog>
#include <QSystemTrayIcon>
#include <QSettings>

И все.

[Алексей1153]

#include <QSettings>
#include <QFileDialog>

- тоже в заголовке не требуются вроде

немного глянул код. Без комментариев мало что сказать можно, разве что не всегда удобное форматирование. Но вот один момент явно не нравится:

void WallWindow::iconActivated(QSystemTrayIcon::ActivationReason reason)
{
    if (reason==3 || reason==2)
    {
        qDebug()<<"Fast swich";
        setImage();
    }
    if (reason==4)
    {

переменная имеет тип QSystemTrayIcon::ActivationReason , а сравниваешь с цифрами-константами, хотя там НАВЕРНЯКА есть именованные константы

[RazrFalcon]

И в правду есть

if (reason==QSystemTrayIcon::Trigger)
    {
        qDebug()<<"Fast swich";
        setImage();
    }
    if (reason==QSystemTrayIcon::MiddleClick)

>>тоже в заголовке не требуются вроде
Нужно для:
QSettings *settings;
и
QFileInfoList, QFileInfo

Сообщение отредактировал RazrFalcon - 19.3.2011, 18:59