Wątek przeniesiony 2018-07-17 08:13 z Newbie przez kq.

Prośba o code review

0

Witam. Mógłby ktoś spojrzeć na mój kod? Wszelkie uwagi dotyczące zarówno działania, jak i stylu pisania kodu mile widziane:). W klasie SQLConnection mam zakomentowany pewien fragment, ponieważ w takiej formie nie działał. Zapytanie wykonywało się tylko wtedy, gdy zamiast '?' wpisywałam konkretną nazwę kolumny. Jeśli ktoś z Was będzie miał pomysł dlaczego nie działa to również prosiłabym o pomoc.
https://github.com/quark001/Calendar

Z gory serdecznie dziękuję.

5
  • using namespace std; w nagłówku!
  • Płaska organizacja projektu (wszystko w głównym katalogu).
  • brak pliku projektu! jakiś cmake, make lub przynajmniej coś charakterystycznego dla IDE jakiego używasz.
  • if (cos) return true; else return false zamiast return cos;.
  • komentarze przy prywatnych metodach a brak komentarzy przy publicznych (co mnie obchodzą prywatne metody, gdy używam klasy)
  • brak użycia/wykorzystania forward declaration, np w SQLConnection.h powinno być tak:
#ifndef CALENDAR2_SQLCONNECTION_H
#define CALENDAR2_SQLCONNECTION_H

#include <string>

// do przeniesienia do pliku cpp
// #include <cppconn/driver.h>
// #include <cppconn/connection.h>
// #include <cppconn/resultset.h>
// #include <cppconn/statement.h>
// #include <cppconn/exception.h>

#include <vector>
// #include "SQLConnection.h" - plik załącza sam siebie !!!

// do przniesienia do cpp
// #include "Activity.h"
// #include "CalendarEntry.h"

// forward declarations
namesapce sql {
    class Driver;
    class Connection;
    class Statement;
    class ResultSet;
    class PreparedStatement;
}

class Activity;
class CalendarEntry;

class SQLConnection
{
public:
	SQLConnection ();
	~SQLConnection ();
	void connect ();
	void closeConnection ();
	void readAllActivitiesToVector ( std::vector < Activity > & );
	void readCalendarEntriesToVector ( std::vector < CalendarEntry > &, string );
	void deleteActivity ( Activity & );
	void addActivity ( Activity & );
	void addCalendarEntry ( CalendarEntry & );
	void updateCalendarEntry ( CalendarEntry &, int );

private:
	sql::Driver *driverPtr;
	sql::Connection *connectionPtr;
	sql::Statement *statementPtr;
	sql::ResultSet *resultSetPtr;
	sql::PreparedStatement *preparedStatementPtr;
};

#endif //CALENDAR2_SQLCONNECTION_H
  • surowe wskaźniki to anachronizm, używaj std::unique_ptr lub std::shared_ptr
  • własna kulawa implementacja czasu MTime oraz Date- używaj std::chrono lub ctime, względnie użyj MTime do opakowania jednego z tych rozwiązań.
  • jest jeszcze parę rzeczy, ale na razie sobie daruję

Żeby nie tylko zrzędzić, chwali się, że

  • jesteś konsekwentny w formatowaniu kodu
  • piszesz małe funkcje
  • jak na początkującego to całkiem nieźle
1
void Menu::help ()
{
	cout
		  << "**********\n";
	cout
		  << "***HELP***\n";
	cout
		  << "**********\n";
	cout
		  << "ENTER:\n";
	cout
		  << "0 - to quit\n";
	cout
		  << "1 - for help\n";
	cout
		  << "2 - to mark calendar entry as done\n";
	cout
		  << "3 - to add a new calendar entry\n";
	cout
		  << "4 - to print calendar entries\n";
	cout
		  << "5 - to add a new activity\n";
	cout
		  << "6 - to remove a activity\n";
	cout
		  << "7 - to print activities\n"
		  << endl;

}

Można to zrobić tak:

void Menu::help()
{
    std::cout <<
    "**********\n"
    "***HELP***\n"
    "**********\n"
    "ENTER:\n"
    "0 - to quit\n"
    "1 - for help\n"
    "2 - to mark calendar entry as done\n"
    "3 - to add a new calendar entry\n"
    "4 - to print calendar entries\n"
    "5 - to add a new activity\n"
    "6 - to remove a activity\n"
    "7 - to print activities\n\n";
}
0

Poniższe hardkodowanie poświadczeń do bazy danych też nie jest super:
https://github.com/quark001/Calendar/blob/master/SQLConnection.cpp

Najlepiej wrzucić to w jakiś plik konfiguracji, z którego będziesz odczytywać. To jest aplikacja desktopowa więc każdy klient będzie miał u siebie, na komputerze jawne dane odnośnie logowania do bazy, w dodatku jako root. Taki plik należałoby wiec odpowiednio zabezpieczyć vide zaszyfrować i w ogóle łączyć się z bazą na dedykowanym użytkowniku, a nie na koncie root'a.

1 użytkowników online, w tym zalogowanych: 0, gości: 1