Skip to content

Hail postgresql database!#2

Open
kupihleba wants to merge 1 commit into
masterfrom
database
Open

Hail postgresql database!#2
kupihleba wants to merge 1 commit into
masterfrom
database

Conversation

@kupihleba

Copy link
Copy Markdown

ODB & ORM

  • Authenticator
    SHA3 hashing, secure comparison, 32 bytes salt
  • MapProvider
    Class for storing playing fields

ODB & ORM

+ Authenticator
  SHA3 hashing, secure comparison, 32 bytes salt
+ MapProvider
  Class for storing playing fields

@leshiy1295 leshiy1295 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Код хорошего качества!
Есть пара небольших замечаний и предложение использовать конфигурационные файлы для подключения к БД. Тут, к сожалению, особо нет паттернов пока что (архитектура довольно простая) - можете внедрить идиому PIMPL, например, а также оценить масштабируемость Вашего решения. Тогда формально Вашу работу можно будет оценить по всем трём обязательным модулям (логика, библиотеки и паттерны).

Comment thread AuthenticationManager.h
#pragma once

#include <string>
#include <boost/tr1/memory.hpp>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

лучше делать так:

  1. в header-файлы не выносить зависимости реализации - сейчас Вы требуете от всех потенциальных клиентов Вашей библиотеки иметь boost и odb, что, кажется, для публичных интерфейсных методов несколько излишне - ведь реализовать их можно и на обычных файлах или даже в памяти...
  2. пользоваться идиомой PIMPL для сокрытия реализации - сейчас же мы видим всю приватную часть интерфейса, а при добавлении нового - придётся перекомпилировать и другие файлы проекта

Comment thread AuthenticationManager.h
private:
friend class User;

std::tr1::shared_ptr <odb::core::database> db;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

зачем здесь используется tr1?

Comment thread AuthenticationManager.cpp

// CONSTRUCTOR //
AuthenticationManager::AuthenticationManager() {
this->db = std::tr1::shared_ptr<database>(new odb::pgsql::database(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

пользуйтесь make_shared

Comment thread AuthenticationManager.cpp
typedef odb::query<User> Query;
typedef odb::result<User> Result;

transaction t(db->begin());

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

зачем это?
и даже если Вы её тут открываете зачем-то, то не закрываете...

Comment thread PrivateData.h
* @attention DEMO ONLY!
*/
namespace PrivateData {
const string DB_USER = "";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

обычно для этого пользуются конфигурационными файлами
как с ними работать - можно, например, посмотреть в лекции про Boost

Comment thread user.hxx
User(const string &name, const string &password)
: name(name), is_admin(false) {

salt = AuthenticationManager::gen_salt();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

можно использовать this->set_password...

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Хм, действительно;)

Comment thread AuthenticationManager.cpp
*/
bool AuthenticationManager::user_exists(const string &name) {
typedef odb::query<User> Query;
typedef odb::result<User> Result;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

а где это используется?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Метод можно будет использовать при регистрации пользователей. Предполагается, что регистрируясь, пользователь может получить информацию, существует ли пользователь с его именем. Оформлен в виде отдельного метода так как есть мысли, как можно будет оптимизировать проверку существования.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

я не про метод - я про typedef

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Понял

Comment thread demo.cxx
cout << elem << '\t';
}
cout << endl;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

если это тест, то следует проводить сравнение с тем, что импортировали :)

Comment thread user.hxx
*/
#pragma db object table("users")

class User {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

сейчас у Вас сущность User жёстко привязана к PSSQL
возможно, стоит выделить отдельный интерфейс и работать через него.
Тут же создать определённого наследника (или скрытую реализацию), работающего именно через PSSQL. Это более расширяемо

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants