Класс User с возможностью выгрузки списка юзеров в файл

Класс User с возможностью выгрузки списка юзеров в файл

Всем доброго времени суток.
Пишу класс User, который хранит в себе имя пользователя и его рекорд в игре. Так же должны быть функции считывания списка пользователей из файла( список — vector<User> ) и запись списка в файл.
Пишу код:

Файл main.cpp — тестирование класса User в форме простенькой БД

#include <iostream>
#include <vector>

using namespace std;

#include "User.h"

int main() 
{ 
    setlocale( 0, "" );
    int n;
    vector<User> list;
    try
    {
        getUserList( list );
    }
    catch ( UserExeption &e )
    {
        e.what();
    }

    while ( true )
    {
        bool ex = false;
        cout << "Состояние списка пользователей на данный момент:" << endl;
        cout << "Количество пользователей: " << list.size() << endl;
        for ( int i = 0; i < list.size(); i++ )
            cout << "Пользователь [ " << i + 1 << " ]:" << endl
                 << "\tИмя: " << list[i].getName() << endl
                 << "\tРекорд: " << list[i].getScore() << endl;

        cout << "\n\nВыберите действие: " << endl;
        cout << "1 - добавление пользователя" << endl;
        cout << "2 - удаление пользователя" << endl;
        cout << "3 - выход" << endl;

        cin >> n;

        switch ( n )
        {
            case 1:
                {
                    string n;
                    long s;
                    cout << "Введите имя пользователя: ";
                    cin >> n;
                    cout << "Его рекорд: ";
                    cin >> s;
                    User add ( s, n );
                    list.push_back( add );
                };break;
            case 2:
                {
                    cout << "Введите номер удаляемого пользователя: ";
                    cin >> n;
                    list.erase( list.begin() + (n-1) );
                };break;
            case 3:
                {
                    ex = true;
                };break;
            default:
                cout << "Что?" << endl;
                break;
        } 
        if ( ex ) 
            break;
    }

    try
    {
        writeUserList( list );
    }
    catch ( UserExeption &e )
    {
        e.what();
    }
    cin.get();
    return 0;
}

Файл User.h — определение класса User

#ifndef _USER_H_
#define _USER_H_

//User - äëÿ ïðåäñòàâëåíèÿ ïîëüçîâàòåëÿ

#include <string>
using std::string;

#include <vector>
using std::vector;

class User
{
    friend void getUserList( vector<User>& );
    friend void writeUserList( const vector<User>& );

    public:
        User( long = 0, const string& = "N/A" );

        void setScore( long );
        void setName( const string& );

        long getScore() const;
        string getName() const;

    private:

        long score;
        string name;
};


enum UserError {
    FILE_NOT_OPEN, BAD_FILE
};
class UserExeption
{
    public:
        UserError error;
        UserExeption( UserError err ): error( err ) {}
        void what()
        {
            switch ( error )
            {
                FILE_NOT_OPEN:
                    cout << "Невозможно открыть файл user_list.dat" << endl;
                    break;
                BAD_FILE:
                    cout << "Ошибка в файле user_list.dat" << endl;
                    break;
            }
        }
};

#endif //_USER_H_

Файл User.cpp — реализация класса User

#include <iostream>
#include <fstream>
#include <string>
#include <vector>

using namespace std;

#include <windows.h>

#include "User.h"

#include "func.h"

User::User( long s, const string &n ): score( s ), name( n )
{
}

void User::setScore( long p )
{
    score = p;
}

void User::setName( const string &n )
{
    name = n;
}

long User::getScore() const
{
    return score;
}

string User::getName() const
{
    return name;
}

void getUserList( vector<User> &userList )
{
    string userListWay = getProgWay() + "\\data\\users\\users_list.dat";
    ifstream file( userListWay.c_str(), ios::in );
    if ( !file.is_open() )
        throw UserExeption( FILE_NOT_OPEN );

    int n;
    file >> n;

    if ( file.fail() )
        throw UserExeption( BAD_FILE );

    for ( int i = 0; i < n; i++ )
    {
        file >> userList[i].score;
        getline( file, userList[i].name, '\n' );
        if ( file.fail() )
            throw UserExeption( BAD_FILE );
    }

    file.close();
}

void writeUserList ( const vector<User> &userList )
{
    string userListWay = getProgWay() + "\\data\\users\\users_list.dat";
    ofstream file( userListWay.c_str(), ios::out | ios::trunc );
    if ( !file.is_open() )
        throw UserExeption( FILE_NOT_OPEN );

    file << userList.size() << endl;

    for ( int i = 0; i < userList.size(); i++ )
        file << userList[i].score << " " << userList[i].name << endl;

    file.close();
}

Примечание: функция getProgWay() возвращает путь к исполняемой программе, определена в func.h.

Программа работает на ура, если файл пуст или его вообще нет. Но совершенно не хочет работать, если в файл уже была запись.
Программа вылетает( вообще полностью ) где-то между объявлением list и циклом while. В чём проблема?

UPD
Вставил более новую версию кода

Программа валится в getUserList() здесь:

    for ( int i = 0; i < n; i++ )
    {
        file >> userList[i].score;   // <-----------
        getline( file, userList[i].name, '\n' );
        if ( file.fail() )
            throw UserExeption( BAD_FILE );
    }

Когда вызывается getUserList(), вектор list не содержит элементов. Следовательно userList[i] обращается к несуществующему элементу, тем более, что для vector операция [] не проверяет валидность индекса, в отличие от vector<>::at(). Пользуйся push_back(), как в main().

И наконец, пользуйся отладчиком!

Не могу пройти мимо чужого огорода, что бы не кинуть пары камней ;-)

(1) Класс User можно совершенно спокойно делать структурой, попутно избавившись от четырёх методов: setScore(), setName(), getScore(), getName(). Закрытые данные и методы геттеры/сеттеры нужны только в том случае, если надо либо закрыть доступ к данным, либо при (как правило) установке данных (метод-сеттер) делается какая-то дополнительная работа. У тебя сейчас всё совершенно прозрачно и вышеперечисленные методы тривиальны. (Может, конечно, ты планируешь в будущем делать в этих методах что-то более сложное...)

(2) В классе User встречаемся с тем же вопросом, который я поднимал в отношении PascalString — сериализация/десериализация. Про PascalString ты мне ответил, что за сериализацию должен отвечать вызывающий код. Вот теперь смотрим, что из этого получается.

Класс User, с моей точки зрения, имеет два кривых костыля:

class User
{
    friend void getUserList( vector<User>& );
    friend void writeUserList( const vector<User>& );

Эти функции работают как с внутренними данными типа User, так и с контейнером. Нехорошо.

Почему в классе не определить методы?

ostream& write(ostream&) const;
istream& read(istream&);

Или не перегрузить для класса User операции << и >>? IMHO это логично и удобно:

// запись
size_type lsize = list.size();
fout << lsize << endl;
for (size_type i = 0; i < lsize; ++i)
    fout << list[i] << endl;


// чтение
size_type lsize = list.size();
User tmp;
fin >> lsize;
for (size_type i = 0; i < lsize; ++i) {
    fin >> tmp;
    list.push_back(tmp);
}

Понятно, что этот код схематичен, чисто для иллюстрации.

Чтение и запись конечно нужно упаковать в функции. Только никаких дружеских отношений эти функции к классу User не должны иметь. Это уже другой уровень абстракции: это — работа с вектором объектов типа User.

(3) Кстати, обрати внимание, что vector<>::size() возвращает отнюдь не int.

(4) Таких вот вещей тоже лучше не делать, тем более, что этот литерал у тебя используется минимум в двух местах.

    string userListWay = getProgWay() + "\\data\\users\\users_list.dat";

Сделай этот литерал именованной константой.

(5) Если уж ты используешь такой мощный класс для исключений, то...

(5.1) Разнеси его на два файла: заголовочный и реализацию (как минимум, метод what()).

(5.2) Если используешь перечисление, то либо пользуйся enum class (C++11), либо объявляй перечисление внутри класса UserExeption. Тогда обращение к члену перечисления будет выглядеть как UserExeption::FILE_NOT_OPEN, но зато гарантированно избежишь конфликта имён.

(6) Функция getUserList() НЕ должна предполагать, что list пустой.

Вот.

И наконец, пользуйся отладчиком!

Я пытался, только копание в плохо понятном мне ассемблерном коде нисколько не помогло. :(

Или не перегрузить для класса User операции << и >>?

В данном случае, не имеет смысла, поскольку чтение и вывод в поток будут производиться всего один раз — в функциях getUserList() и writeUserList().

Класс User можно совершенно спокойно делать структурой

То есть как-то так?:

User.h

#ifndef _USER_H_
#define _USER_H_

//User - äëÿ ïðåäñòàâëåíèÿ ïîëüçîâàòåëÿ

#include <string>
using std::string;

#include <vector>
using std::vector;

struct User
{
    long score;
    string name;
};

void getUserList( vector<User>& );
void writeUserList( const vector<User>& );


class UserExeption
{
    public:
        enum {
            FILE_NOT_OPEN, BAD_FILE
        };
        int error;
        UserExeption( int err ): error( err ) {}
    void what();

};

#endif //_USER_H_

и файл User.cpp

#include <iostream>
#include <fstream>
#include <string>
#include <vector>

using namespace std;

#include <windows.h>

#include "User.h"

#include "func.h"

const string usersListWay = getProgWay() + "data\\users\\users_list.dat";

void getUserList( vector<User> &userList )
{
    ifstream file( usersListWay.c_str(), ios::in );
    if ( !file.is_open() )
        throw UserExeption( UserExeption::FILE_NOT_OPEN );

    userList.clear();

    int n;
    file >> n;

    if ( file.fail() )
        throw UserExeption( UserExeption::BAD_FILE );

    for ( int i = 0; i < n; i++ )
    {
        User temp;
        file >> temp.score;
        getline( file, temp.name, '\n' );
        if ( file.fail() )
            throw UserExeption( UserExeption::BAD_FILE );

        userList.push_back( temp );
    }

    file.close();
}

void writeUserList ( const vector<User> &userList )
{
    ofstream file( usersListWay.c_str(), ios::out | ios::trunc );
    if ( !file.is_open() )
        throw UserExeption( UserExeption::FILE_NOT_OPEN );

    file << userList.size() << endl;

    for ( int i = 0; i < userList.size(); i++ )
        file << userList[i].score << " " << userList[i].name << endl;

    file.close();
}

void UserExeption::what()
{
    switch ( error )
    {
        case FILE_NOT_OPEN:
            cout <<  "Íå óäàëîñü îòêðûòü ôàéë users_list.dat" << endl;
            break;

        case BAD_FILE:
            cout <<  "Ôàéë users_list.dat ïîâðåæä¸í"  << endl;
            break;

        default:
            cout <<  "Íåïðåæâèäåííàÿ îøèáêà"  << endl;
            break;
    }
}

сли используешь перечисление, то либо пользуйся enum class (C++11), либо объявляй перечисление внутри класса UserExeption.

А разве это не будет равносильно:

class UserExeption
{
   public:
      const int FILE_NOT_OPEN = 1;
      const int BAD_FILE = 2; 
      int err;
      UserExeption( int err ): error( err ) {}
};

То есть будут потери памяти. Или нет?

И наконец, пользуйся отладчиком!

Я пытался, только копание в плохо понятном мне ассемблерном коде нисколько не помогло. :(

Я тебя не призываю копаться в окне ассемблерного кода, не зная ассемблера )) Но тебе ни что не мешает расставить точки останова в исходном коде на С++, при остановах посмотреть значения переменных, установить наблюдение за переменными, пошагово пройти подозрительные места и пр.

Тогда у тебя не будет возникать вопросов типа у меня программа крашится где-то между Москвой и Воронежем...

Кстати, в плане отладки Code::Blocks сделан получше, чем Dev-C++. Хотя оба используют и компилятор, и отладчик из MinGW. (Я, например, к Code::Blocks прикрутил компилятор, входящий в комплект поставки Dev-C++.)

Или не перегрузить для класса User операции << и >>?

В данном случае, не имеет смысла, поскольку чтение и вывод в поток будут производиться всего один раз — в функциях getUserList() и writeUserList().

С точки зрения идеологии ООП — совершенно необдуманное заявление. Объект должен сам обслуживать свои данные и следить за их целостностью.

Класс User можно совершенно спокойно делать структурой

То есть как-то так?:

Ну да, примерно так. Я бы, единственно, оставил конструктор по умолчанию — не люблю неинициализированные переменные. И, см. выше, добавил бы перегруженные операции << и >>.

Интересно, что при чтении из файла ты проверяешь каждый чих и выдаёшь, если надо исключения, а при записи файла считаешь, что если файл успешно открыт, то и запись пройдёт без проблем.

И вот это

const string usersListWay = getProgWay() + "data\\users\\users_list.dat";

выглядит опасно, поскольку функция getProgWay() будет вызываться до начала работы main(). Я бы оставил так:

const string usersListPartialWay = "data\\users\\users_list.dat";

а формирование полного имени файла делал в main() или каком-то другом подходящем месте (например в функции, которая делает первичную инициализацию игры).

А разве это не будет равносильно:
[...]
То есть будут потери памяти. Или нет?

Не будет равносильно. В твоём варианте каждый экземпляр класса UserExeption будет иметь два константных члена типа int. А когда используется перечисление (enum), символические имена в нужных местах заменяются компилятором на непосредственные значения. Т.е. объявление enum En { E1 = 0, E2, E3, E4 = 100500 ... } кода вообще не генерирует. В этом смысле, аналогичное поведение у #define E4 100500, только #define обрабатывает препроцессор, а enum — компилятор.

Hint. Что бы при copy-paste из Dev-C++ кириллические символы не превращались в кракозябры, перед копированием переключись в Dev-C++ на ввод кириллицы.

(например в функции, которая делает первичную инициализацию игры).

Но эта функция инициализации игры находится находится слишком далеко от реализации User. Как быть?

выглядит опасно, поскольку функция getProgWay() будет вызываться до начала работы main().

А почему опасно? У меня main только и делает, что объявляет объект и вызывает его метод.

выглядит опасно, поскольку функция getProgWay() будет вызываться до начала работы main().

А почему опасно? У меня main только и делает, что объявляет объект и вызывает его метод.

Аргументированно ответить пока не могу. Но я бы так делать не стал. На уровне интуиции.

На код getProgWay() можно посмотреть?

Но эта функция инициализации игры находится находится слишком далеко от реализации User. Как быть?

А в чём проблема? User у тебя вообще лежит в двух отдельных исходниках. Оно ото всего одинаково далеко ))

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

На код getProgWay() можно посмотреть?

string getProgWay()
{
    char buff[256];
    if ( !GetModuleFileName( NULL, buff, 256 ) )
    {
        cout << "Ошибка: GetModuleFileName( NULL, buff, 256 ) возвратила 0." << endl;
        cin.get();
        exit( EXIT_FAILURE );
    }
    string progWay = buff;
    int i;
    for ( i = progWay.size() - 1; progWay[i] != '\\' && i >= 0; i-- )
        ;
    if ( i )
    {
        progWay.resize( i );
        progWay += '\\';
    } 
    return progWay;
}

А так не проще вместо вызова GetModuleFileName() использовать argv[0]?

Ну дак много лишней мороки будет с передачей этого argv[0]. А так всё просто.

getProgWay

No way :) Скорее GetProgPath, или GetPath(). Используй argv[0], как советовал Череп, оно хотя-бы кроссплатформенное.

Могу предложить создать отдельный класс логгера, туда передать полный путь к argv[0] и вызывать ее там, где нужно. Потом ты сможешь реализовать удаленное логирование, либо через системный журнал, не меня вызовы записи логов по всей игре.

Exeption

Опять же, правильно — Exception. И лучше реализовать собственное исключение через расширение стандартного:

class user_exception : public std::runtime_error
{
public:
    explicit user_exception(const std::string& what_arg) : std::runtime_error(what_arg) {};
    virtual ~user_exception() throw() {};
};

оно хотя-бы кроссплатформенное.

в игре столько вещей привязанных к платформе windows, что GetModuleFileName является лишь каплей в море...

И лучше реализовать собственное исключение через расширение стандартного:

А что это даёт?

А что это даёт?

Все исключения в C++ наследуются от std::exception. У него есть стандартизированный интерфейс, метод what() и т.п.

Во первых, это избавляет тебя от написания собственной реализации, во вторых делает твой код мягким и шелковистым для других программистов :)

Кстати, логирования — такая штука, где параллельность просто необходима. Попробуй написать асинхронный логгер, который не будет тормозить игру, записывая данные на диск в отдельном потоке. Сделать его потокобезопасным (т.е., чтобы один объект мог использоваться параллельно в разных участках кода). Можно также сделать очередь сообщений и разделить типы логов (ошибки и отладочная информация), сделав возможно записывать два лога параллельно.

Книжку читал, что сбрасывал Croessmah?

Ещё нет, читаю Дейтел Х. Дейтел П — Как программировать на С++. :)

Внимание! Это довольно старый топик, посты в него не попадут в новые, и их никто не увидит. Пишите пост, если хотите просто дополнить топик, а чтобы задать новый вопрос — начните новый.

Ответить

Вы можете использовать разметку markdown для оформления комментариев и постов. Используйте функцию предпросмотра для проверки корректности разметки.

Пожалуйста, оформляйте исходный код в соответствии с правилами разметки. Для того, чтобы вставить код в комментарий, скопируйте его в текстовое поле ниже, после чего выделите то, что скопировали и нажмите кнопку «код» в панели инструментов. Иначе ваш код может принять нечитаемый вид.

Либо производите оформление кода вручную, следующим образом:

``` #include <iostream> using namespace std; int main() { // ... } ```

Предпросмотр сообщения

Ваше сообщение пусто.