Подключение к базе данных MySQL


Я бы создал класс C# для подключения к базе данных MySQL (подключение.CS) и отдельный класс для проверки пользователя (пользователей.НЦБ).

Связи.в CS

public static MySqlConnection GetConnection()
{
    try
    {
        var connectionString = ConfigurationManager.ConnectionStrings["ConnectionString"].ConnectionString;
        _connection = new MySqlConnection(connectionString);
        _connection.Open();
    }
    catch (MySqlException ex) {

        switch (ex.Number)
        {
            //0: Cannot connect to server.
            case 0:
                MessageBox.Show("Cannot connect to server.  Contact administrator");
                break;
            //1045: Invalid user name and/or password.
            case 1045:
                MessageBox.Show("Invalid username/password, please try again");
                break;
        }

    }

    return _connection;
}

Пользователей.в CS

public bool AuthenticateUser()
{
    string query = "SELECT * FROM users WHERE user_name = @userName LIMIT 1";
    bool password = false;
    try
    {
        Console.Write("connection");
        using (MySqlConnection connection = Connection.GetConnection())
        {
            Console.Write("Cmd");
            using (var cmd = new MySqlCommand(query, connection))
            {
                cmd.Parameters.AddWithValue("@userName", this.UserName);
                using (var reader = cmd.ExecuteReader())
                {
                    while (reader.Read())
                    {
                        password = BCrypt.Net.BCrypt.Verify(this._password + reader["salt"].ToString(), reader["password"].ToString());
                        this._user_id = int.Parse(reader["id"].ToString());
                    }
                }
            }
        }
    }
    catch (MySqlException ex)
    {
        throw new ArgumentException(ex.Message);
    }
    catch (Exception ex) {
        throw new ArgumentException(ex.Message);
    }

    return password;

}

Я не создан способ распоряжаться/закрыть соединение больше, так как, насколько мне известно, using заявление автоматически освобождает типа IDisposable. Я ищу предложение прежде чем я продолжу кодирования дополнительные функции.



1714
2
задан 28 февраля 2018 в 12:02 Источник Поделиться
Комментарии
3 ответа

Пару вопросов я нашел:

GetConnection метод:


var connectionString = ConfigurationManager.ConnectionStrings["ConnectionString"].ConnectionString;
_connection = new MySqlConnection(connectionString);
_connection.Open();

Здесь вы создаете новое соединение объекта и сохранения его в _connection. Это не определено нигде в коде, но, кажется, как частное поле. Это не должно быть необходимости, так как вы просто вернуть его и оставить его абоненту, чтобы справиться с этим. Просто использовать обычную переменную и избавиться от поля:

MySqlConnection connection = new MySqlConnection(connectionString);
connection.Open();
return connection;


    catch (MySqlException ex) {

switch (ex.Number)
{
//0: Cannot connect to server.
case 0:
MessageBox.Show("Cannot connect to server. Contact administrator");
break;
//1045: Invalid user name and/or password.
case 1045:
MessageBox.Show("Invalid username/password, please try again");
break;
}

}


Это выглядит как плохая идея для меня. Например, вы используете MessageBox.Show для отображения ошибок, которая связывает ее с уровня представления. Это нарушает разделение, так как класс должен делать только одну вещь. Оставить его абоненту для отображения ошибок, как он любит.

Другой вопрос, что каких-либо дополнительных подробностей об ошибке пропало, как трассировка стека, сообщение или какая-либо внутренняя исключения. А во-вторых, главная проблема заключается в том, что любая ошибка с числом, кроме 0 или 1045 просто обмелел и абонент не имеет понятия, он не смог, оставив его с закрыты, ошибка подключения. Мое предложение-просто удалите try-catch выполняет все и делают вызывающий обработки ошибок, как он сочтет целесообразным.

AuthenticateUser метод:


string query = "SELECT * FROM users WHERE user_name = @userName LIMIT 1";

Небольшая проблема с LIMIT 1 это означает, что не может быть более одного пользователя в БД с тем же именем. Что может быть указанием на ДБ конструкция, отсутствует UNIQUE ключ в user_name колонка, она не должна быть нужна. Еще одной проблемой является использование SELECT * который приносит больше данных, чем необходимо. Указать те столбцы, которые вы на самом деле использовать и оставить все остальное.


Console.Write("connection");

Это странно ставить здесь. Для одного, другие части кода, обратитесь к MessageBox для отображения ошибок и сообщений, при этом используется консоль. Почему такая непоследовательность? Либо использовать одну технологию дисплея или другой, но явно не так. Как и прежде, это создает плохое впечатление пользователей в случайном отладочных сообщений из ничего. Скорее всего, это может быть заменен на какой-то лог-файл или похожие, но помните, чтобы добавить больше контекста к нему, так что вы можете извлечь полезную информацию из файла.


cmd.Parameters.AddWithValue("@userName", this.UserName);

Это всегда приятно видеть правильно параметризованных запросов. Но помните, что AddWithValue это плохая практика, объявить точный тип данных для параметра и убедитесь, что он соответствует серверной стороне декларации точно.


while (reader.Read())

Нет необходимости для петли здесь. Как и раньше, запрос будет возвращать только 1 строку или имя пользователя неправильно, так что просто заменить его на один звонок. if (!reader.Read()) return false; пойдет, и выйдет раньше ни один пользователь не существует.


password = BCrypt.Net.BCrypt.Verify(this._password + reader["salt"].ToString(), reader["password"].ToString());

Приятно видеть правильно хранить пароль и хорошая проверка! Хотя есть одна маленькая вещь. Держите область password переменная как можно меньше, возможно, вернуться в этот момент, чтобы избежать необходимости читать до конца метода. Кроме того, "пароль" не кажется правильное название для этого, что-то вроде "validPassword" бы лучше описать то, что происходит.


this._user_id = int.Parse(reader["id"].ToString());

Это, возможно, самая серьезная проблема. Метод использует поля класса все другие места (ввести имя пользователя и пароль и возвращает идентификатор пользователя), который кажется странным для меня, потому что связи в результате этого метода на состояние класса, которое можно использовать в другом месте (хотя мы не так много контекста). Почему бы не использовать вместо параметров методов и возвращаемых значений? Так что вы можете избавиться от всех частных переменных на уровне класса. Я нахожу такой декларации способ значительно лучше:

public int? AuthenticateUser(string userName, string password)

Имя пользователя и пароль передаются в качестве параметров, а не поля класса, а возвращаемое значение-это идентификатор пользователя или null если ничего не найдено. Это ясно выражает то, что происходит, и что метод ожидает и возвращения. Кроме того, это позволяет упростить тела, делая что-то вроде этого:

//Check if the password matches, and exit if not
if(!BCrypt.Net.BCrypt.Verify(this._password + reader["salt"].ToString(), reader["password"].ToString())) return null;
//We've got a valid user and password, return its id
return int.Parse(reader["id"].ToString());


   catch (MySqlException ex)
{
throw new ArgumentException(ex.Message);
}

Опять та же проблема. Исключение перехватывается и выдается. Но здесь проблема в том, что он вводит в заблуждение. С одной стороны, тип ArgumentException это неправильно, вы не ошиблись в рассуждениях, что-то взорвали при обращении к БД. Кроме того, вы останетесь только в сообщение, но вы откажитесь от трассировки стека, любое внутреннее исключение и ДБ-определенное исключение информации, которая может помочь отладки. Это, вероятно, даст вам проблем в дальнейшем. Мое предложение состоит в том, чтобы просто удалить попробовать-поймать и оставить его абоненту для обработки исключения и журнала, показывают что-нибудь или повторить. В крайнем случае, кинуть конкретную исключением добавления некоторых условиях, но и сохранить все исходное исключение как внутреннее исключение.

2
ответ дан 4 марта 2018 в 11:03 Источник Поделиться

Метод возвращает интерфейс IDisposable Объект (MySqlConnection), поэтому вы правы в том, что вам не нужно беспокоиться об утилизации соединение при использовании Using заявление.


Насколько вы используете connectionString вы на самом деле не нужно это здесь:


try
{
var connectionString = ConfigurationManager.ConnectionStrings["ConnectionString"].ConnectionString;
_connection = new MySqlConnection(connectionString);
_connection.Open();
}

Просто положите строку в качестве параметра

try
{
_connection = new MySqlConnection(ConfigurationManager.ConnectionStrings["ConnectionString"].ConnectionString);
_connection.Open();
}

это делает эту линию немного дольше, но не тратить время присвоения переменной, а затем вызвать значение переменной. Если вы используете его больше, чем просто прямо здесь, я согласен, что вы должны присвоить его переменной.

Ради удобочитаемости, вы можете сделать это также.

try
{
_connection = new MySqlConnection(
ConfigurationManager
.ConnectionStrings
["ConnectionString"]
.ConnectionString
);
_connection.Open();
return _connection;
}

Я также перенес возвращение так, чтобы он был внутри оператора try, нет никаких причин, чтобы оставить его там, все само по себе. вы могли бы сделать исключение на возвращение, выданному бы не быть SQL-исключение, но это еще хорошо, чтобы получить в привычку держать код вместе, ловить не отдельные это может, там, кажется, не быть причиной для этого.

1
ответ дан 28 февраля 2018 в 03:02 Источник Поделиться

Вы должны нулевое значение user_id

bool password = false;
this._user_id = null;

match было бы лучше, чем имя password

Вы должны выбрать имена столбцов, а не * и относятся к ним по порядковому номеру с целью повышения эффективности.

0
ответ дан 28 февраля 2018 в 02:02 Источник Поделиться