1
\$\begingroup\$

In my Server class, I have a few methods that handle different queries to be sent to the database. Everything works fine, but I really want to DRY up this class, because I feel like it could be much better written than it is now.

The class is designed to strictly handle all client to server interactions by having all parameters encrypted and formatted prior to reaching this class. Each method instantiates a new MySqlConnection and MySqlCommand using a specified encrypted string from the Constants class, which then decrypts it for use. Aside from the GetInfo() method, which is used on startup to retrieve, decrypt and then insert client information into an array for later usage, all methods take some number of parameters. While UploadUserData() and CheckIfExists() only use a single object, UploadScreen() and UpdateScreen() each take three parameters.

Other than handling the MySqlConenction instance, I'm a little stuck on how I should go about cleaning up the rest of my code, as I'm still relatively new to this. Any ideas would be much appreciated.

I was wondering how I could go about returning a new MySqlCommand under a single method, even though some commands take different parameters (object, object[] or none). I'm also looking to make the class itself more universal with command execution rather than having to explicitly call on the method it falls under in order to do what I need it to do.

I'm using an assembly called MySQL Connector/Net that contains the classes MySqlConnection, MySqlCommand and MySqlDataReader, which is used at cmd.ExecuteReader().

internal static class Server
{
    internal static string[] Info;

    private static MySqlConnection SqlConnection(string connection)
    {
        var connect = new MySqlConnection(Crypto.Decrypt(connection));
        try
        {
            connect.Open();
        }
        catch (Exception)
        {
            throw new Exception("Error connecting to server");
        }
        return connect;
    }
    internal static bool GetInfo()
    {
        try
        {
            using (var command = new MySqlCommand(Crypto.Decrypt(Constants.SelectInfoQuery), SqlConnection(Constants.ConnectSelect)))
            using (var data = command.ExecuteReader())
            {
                while (data.Read())
                    Info = Regex.Split(Crypto.Decrypt(data[1].ToString()), ";");
                return true;
            }
        }
        catch (MySqlException ex)
        {
            Console.WriteLine(ex);
            return false;
        }
        finally
        {
            MySqlConnection.ClearAllPools();
        }
    }

    internal static void UploadUserData(object parameters)
    {
        try
        {
            using (var cmd = new MySqlCommand(string.Format(Crypto.Decrypt(Constants.InsertStatsQuery), parameters), SqlConnection(Constants.ConnectInsert)))
            {
                cmd.ExecuteNonQuery();
            }
        }
        catch (Exception e)
        {
            Console.WriteLine(e);
        }
        finally
        {
            MySqlConnection.ClearAllPools();
        }
    }

    internal static string CheckIfExists(object parameters)
    {
        try
        {
            using (var cmd = new MySqlCommand(string.Format(Crypto.Decrypt(Constants.CountScreenQuery), parameters), SqlConnection(Constants.ConnectSelect)))
            using (var data = cmd.ExecuteReader())
            {
                data.Read();
                return data[0].ToString() == "1" ? Constants.UserFound : Constants.UserNotFound;
            }
        }
        catch (Exception)
        {
            MessageBox.Show(Constants.ErrorConnection, Constants.ApplicationName, MessageBoxButtons.OK, MessageBoxIcon.Stop);
            return null;
        }
        finally
        {
            MySqlConnection.ClearAllPools();
        }
    }

    internal static void UploadScreen(object[] parameters)
    {
        try
        {
            using (var str = new FileStream(Constants.PicturePath, FileMode.Open, FileAccess.Read))
            using (var br = new BinaryReader(str))
            using (var cmd = new MySqlCommand(string.Format(Crypto.Decrypt(Constants.InsertScreenQuery), parameters), SqlConnection(Constants.ConnectInsert)))
            {
                cmd.Parameters.AddWithValue("@var", br.ReadBytes((int) str.Length));
                cmd.ExecuteNonQuery();
            }
        }
        catch (Exception)
        {
            MessageBox.Show(Constants.ErrorScreenShot, Constants.ApplicationName, MessageBoxButtons.OK, MessageBoxIcon.Stop);
        }
        finally
        {
            MySqlConnection.ClearAllPools();
            File.Delete(Constants.PicturePath);
        }
    }

    internal static void UpdateScreen(object[] parameters)
    {
        try
        {
            using (var str = new FileStream(Constants.PicturePath, FileMode.Open, FileAccess.Read))
            using (var br = new BinaryReader(str))
            using (var cmd = new MySqlCommand(string.Format(Crypto.Decrypt(Constants.UpdateScreenQuery), parameters), SqlConnection(Constants.ConnectSelect)))
            {
                cmd.Parameters.AddWithValue("@var", br.ReadBytes((int) str.Length));
                cmd.ExecuteNonQuery();
            }
        }
        catch (Exception)
        {
            MessageBox.Show(Constants.ErrorScreenShot, Constants.ApplicationName, MessageBoxButtons.OK, MessageBoxIcon.Stop);
        }
        finally
        {
            MySqlConnection.ClearAllPools();
            File.Delete(Constants.PicturePath);
        }
    }
}

CONSTANTS (Decrypted):

internal const string ConnectSelect = "server=SERVERIP;database=DATABASE;user id=USER1;password=PASSWORD1";
internal const string ConnectInsert = "server=SERVERIP;database=DATABASE;user id=USER2;password=PASSWORD2";
internal const string SelectInfoQuery = "SELECT * FROM TABLE1";
internal const string CountScreenQuery = "SELECT COUNT(*) FROM TABLE2 WHERE COLUMN1 = '{0}'";
internal const string UpdateScreenQuery = "UPDATE TABLE2 SET COLUMN2 = @var, COLUMN3 = '{1}', COLUMN4 = '{2}' WHERE COLUMN1 LIKE '{0}'";
internal const string InsertScreenQuery = "INSERT INTO TABLE2 (COLUMN1, COLUMN2, COLUMN3, COLUMN4) VALUES ('{0}', @var, '{1}', '{2}')";
internal const string InsertStatsQuery = "INSERT INTO TABLE3 (COLUMN1) VALUES ('{0}')";

Tables, columns and other bits of data were edited since the original strings contain server/database info. For the connect strings, I use two different usernames, as user1 is only allowed to select and update, while user two is only allowed to insert.

\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review. So that we may advise you properly, could you please include the MySQLCommand class and the relevant Constants? Also, could you explain the purpose of the Crypto class, and how it manages to decrypt without specifying a key? \$\endgroup\$ – 200_success Sep 11 '16 at 8:24
  • \$\begingroup\$ The MySQLcommand class comes from an assembly called MySQL Connector/Net. The purpose for the Crypto class being used here is meant to decrypt the const strings in the Constants class, which the key and IV are hard coded in its Decrypt method for now. Do you need me to specify what each Constants.string is though? And thanks for the welcome! \$\endgroup\$ – Chris P. Bacon Sep 11 '16 at 8:33
  • \$\begingroup\$ Please do. We could probably provide better advice on DRYing up code if we could actually see what the queries have in common with each other. \$\endgroup\$ – 200_success Sep 11 '16 at 8:35
  • \$\begingroup\$ @ChrisP.Bacon I assume you're going to remove that hardcode from the Crypto class before releasing this? \$\endgroup\$ – EJoshuaS Sep 12 '16 at 20:57
  • \$\begingroup\$ @EJoshuaS Yes, it's baked in merely for testing purposes and it won't be stored within the client upon release. \$\endgroup\$ – Chris P. Bacon Sep 12 '16 at 22:26

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.