C++/Qt/SQL - kwestia poprawności kodu

poczytaj:
http://qt-project.org/doc/qt-4.8/qsqldatabase.html
http://qt-project.org/doc/qt-4.8/qsqlquery.html
http://en.wikipedia.org/wiki/SQL_injection

potem zadaj pytania jeszcze raz, jeżeli w ogóle bedzie to konieczne

@gośćabc - rozumiem, że twoje 'wskazówki' dotyczą przede wszystkim SQL injection ? Niestety nie wiele znalazłem tam informacji na temat uniknięcia sql-injection w Qt, ale sam znalazłem inne źródło ( https://www.ics.com/designpatterns/book/qsqldatabase.html ) i znalazłem informację, iż aby uniknąć wstrzykiwania kodu należy stosować metodę bindValue() dla używanych kolumn, co też czynie w zamieszczony w pierwszym poście:

 Query.bindValue(i,param[i]); 

Jeżeli chodziło Tobie o coś innego to proszę o doprecyzowanie :) Poza tym zadaje moje pytania jeszcze raz i nadal nie znam na nie odpowiedzi.. :@

w ogóle nie przeczytałeś tego co Ci podlinkowałem, "in which malicious SQL statements are inserted into an entry field for execution", "With most development platforms, parameterized statements that work with parameters can be used (sometimes called placeholders or bind variables) instead of embedding user input in the statement." to z linku 3

czemu poruszyłem temat sql injection, bo Ty to u siebie robisz; w skrócie pozwalasz, aby input z zewnątrz wylądował w kwerendzie bez weryfikacji z bazą danych (jaki typ ma to pole w bazie danych), ktoś kto będzie chciał Ci zaszkodzić, wprowadzi np. login DROP DATABASE i do widzenia z Twoją bazą;

w linku 2 masz obiekt odpowiedzialny za tworzenie i wykonywanie podanych kwerend; SA TAM PRZYKŁADY

a w linku 1 jak utworzyć połączenie do różnych baz danych

edit:
jeżeli nie znasz angielskiego to umieść to w profilu czy coś, bo to tylko komplikuje sprawy; a ja w języku polskim programowania nie tykam

edit:
sql injection tu nie ma sorx, nie czytałem więcej niż 1 funkcję Twojego przykładu co mnie sprowokowało do takiej a nie innej odpowiedzi

mikajlo napisał(a):

1. Wydaje mi się, że tutaj powinno się raczej stosować typ INT, bo tego typu jest kolumna w bazie, ale z drugiej strony wszędzie trzeba operować na stringach więc i tak trzeba rzutować..
   Qt zwraca Ci QVariant. Nie przekształcaj go w QStringa.

mikajlo napisał(a):

1. Czy w takich sytuacja, w c++ argumenty funkcji powinno przekazywać się przez referencje ? (deklaracja funkcji z &query, &arg ) ?
   QString składa się wyłącznie z d-pointera, więc teoretycznie kopia sprowadzi się tylko do inkrementacji refcounta, ale ogółem, jeśli nie zamierzasz zmieniać tej wartości to przekazanie przez const ref jest wskazane (QString const&).

mikajlo napisał(a):

1. Jak zbudowalibyście metodę wyciągającą dane z bazy danych - chodzi konkretnie o select'a ?

QList<QStringList> DataBaseOperation::Select(QString query, QStringList param)

Ja zdecydowałem się wykorzystać typ QList<QStringList>, gdzie QStringList reprezentują kolejne wiersze, natomiast QList jest kontenerem (tabelą) dla tych wierszy.. Zdaje to egzamin, ale coś nie 'pasi' mi przy przekazywaniu tego typu między klasami/metodami.. bo praktycznie jest on przekazywany przez wartość.. (btw. czy w tym przypadku to jest złe ?). Chciałem wykorzystać tutaj wskaźniki, ale zauważyłem że byłby problem z prawidłowym zwalnianiem pamięci.. (przynajmniej jeśli zachowałbym kod w takiej postaci jak jest..)

podsumowując: szukam podpowiedzi mających na celu wzniesienie tego kod na wyższy (odpowiedni) poziom...
Pytasz o konstrukcję metody, a masz na myśli typ zwracany. Zapraszam do dokumentacji: http://qt-project.org/doc/qt-5/qlist.html#QList-2

QList::QList(const QList<T> & other)
Constructs a copy of other.

This operation takes constant time, because QList is implicitly shared.

Jak widać, przekazywanie tego jest stosunkowo tanie (znów refcount).

Zwróciłbym kontener<kontener<QVariant>>, gdzie kontener to std::vector lub QList, w zależności od potrzeb. Jeśli chodzi o samą metodę, to nie widzę specjalnie potrzeby aby istniała - QSqlQuery ma exec, a zbudować kwerendę i tak musisz (a bindValue czytelniej i bezpieczniej używać z parametrami po nazwach, nie po indeksach)

gośćabc napisał(a):

w ogóle nie przeczytałeś tego co Ci podlinkowałem, "in which malicious SQL statements are inserted into an entry field for execution", "With most development platforms, parameterized statements that work with parameters can be used (sometimes called placeholders or bind variables) instead of embedding user input in the statement." to z linku 3

czemu poruszyłem temat sql injection, bo Ty to u siebie robisz; w skrócie pozwalasz, aby input z zewnątrz wylądował w kwerendzie bez weryfikacji z bazą danych (jaki typ ma to pole w bazie danych), ktoś kto będzie chciał Ci zaszkodzić, wprowadzi np. login DROP DATABASE i do widzenia z Twoją bazą;
Mam wrażenie, że nie przeczytałeś jego kodu.

Jedyne miejsce, gdzie @mikajlo przypisuje wartość do kwerendy to:

    for (int i=0; i<param.count(); i++)
        Query.bindValue(i,param[i]);

i robi to w bezpieczny (przynajmniej od strony SQL Injection) sposób.

@mikajlo: jeszcze jedna pierdółka (i raczej sprawa opinii, a nie poprawności): w zdecydowanej większości dobrego kodu z jakim mam styczność, jedyne co zaczyna się wielką literą - jeśli cokolwiek - to nazwy typów (no i makra, ale one są całe wielkimi), natomiast zmienne zaczynają się małą literą. To tak odnośnie Twojego Query.

Witam ponownie..
przerobiłem kod według wskazówek @kq , nie wszystkich.. ale o tym na końcu w pytaniach..

Kod:

Mam klasę DataBaseOperation, która odpowiedzialna jest za komunikację z bazą danych..

 
class DataBaseOperation;

//metoda wyciągająca dane produktow o konkretnej wartosci
QList< QList<QVariant> > DataBaseOperation::getValue(QVariant const& value)  
{                                                                                          
    QString query = QString("SELECT * FROM Product WHERE value= ?");

    QList<QVariant> arg;
    arg.append(value);

    QList<QStringList> result = this->Select(query, arg);

    return result;
}

// ogólna metoda wykorzystywana do wykonywania select'ów do bazy danych..
QList< QList<QVariant> > DatabaseOperationsClass::Select(QString const& querytext, QList<QVariant> const& arg)
{
    QSqlQuery query;
    query.prepare(querytext);
    for (int i=0; i<arg.count();i++)
        query.bindValue(i,arg.at(i));

    if (!query.exec()) {
        qDebug() << query.lastError().text();
    }

    QList< QList<QVariant> > tab;

    if (query.size() > 0 )
    {
        while (query.next())
        {
           QList<QVariant> record;
           for(int i=0; i< query.record().count(); ++i)
              record.append(query.value(i));

           tab.append(record);
        }
    }
    else
    {
        qDebug() << "error";
    }

    return tab;
}

Klasa ValueWidget wyświetlająca dane osoby np. w QEditLine'ach..

 
class ValueWidget;

DataBaseOperation db;
QList< QList<QVariant> > records2 = db.getValue(QVariant value);

1. Przebudowałem metody select'ów, aby zwracały QList<QList <QVariant> >. Jednak nie wiem czy dalej dobrze rozumiem..

kq napisał(a):

Jeśli chodzi o samą metodę, to nie widzę specjalnie potrzeby aby istniała - QSqlQuery ma exec, a zbudować kwerendę i tak musisz (a bindValue czytelniej i bezpieczniej używać z parametrami po nazwach, nie po indeksach)

Czy chodziło Tobie o tego typu konstrukcję:

     QSqlQuery query;
     query.prepare("INSERT INTO person (id, forename, surname) "
                   "VALUES (:id, :forename, :surname)");
     query.bindValue(":id", 1001);
     query.bindValue(":forename", "Bart");
     query.bindValue(":surname", "Simpson");
     query.exec();
 

Tylko o ile dobrze widzę, w takiej sytuacji każda metoda (np. selectPerson, selectValue, itd..) musiałaby zawierać QSqlQuery i dość rozbudowane bindowanie, tak ? W ten sposób zyskuje się na czytelności, ale z drugiej strony stosunkowo dużo kodu jest powielane... czy może się mylę, i tutaj akurat 'wskazane' jest powielanie tego kodu dla uzyskania lepszej czytelności.. ?

1. I jeszcze jedno 'upewniające' mnie pytanie... Rozumiem, że stosowanie QVariant powinno zaoszczędzić trochę mocy obliczeniowej i w końcowym rozrachunku przyspieszyć wykonywanie zapytań, ponieważ nie będzie wymagane niepotrzebne rzutowanie między QVariant <--> QString. Ale pewnie to za bardzo 'kosztowne obliczenia' nie są.. (tak mi się wydaje przynajmniej..), czy są jeszcze jakieś korzyści stosowanie QVariant? (bo może to i mało znaczący szczegół, ale przyjemniej debuggowało (qDebug) mi się całe QStringList'y jako wiersze .. :P