snippetcMinor
Is my MySQL library wrapper neat? How can I do better?
Viewed 0 times
neatcanbetterwrappermysqllibraryhow
Problem
I'm moderately new to programming in C and I'm not sure about programming practices with code; suddenly I have had fears that my code is messy and disorganized. I can read it when I come back a month later yet I'm still nervous if I'm doing anything that's considered taboo.
This isn't all the code that I've done with this project, I think my other code is fine.
```
#include
#include
#include
#include
#include "functions.h"
#include "errh.h"
#include "mysqlerr.h"
void FIRST_TIME_HANDLER(MYSQL *conn)
{
printw("[+] Doing initialization checks...\n");
SEND_MYSQL_QUERY(conn, "create table IF NOT EXISTS user_2(username varchar(25) not null, passwd varchar(25) not null)");
printw("[+] Handler has checked tables.\n");
refresh();
}
void INThandler(int sig)
{
signal(sig, SIG_IGN);
printw("Closing server...\n");
refresh();
sleep(5);
endwin();
exit(0);
}
MYSQL_INFO *CONNECTION_INIT()
{
MYSQL_INFO info = malloc(sizeof(info));
info->IP = "IPADDRGOHERE"; info->user = "Hello";
info->pass = "World"; info->DB = "DBGOHERE";
return info;
}
int EXIT_SERVER(MYSQL *connection)
{
mysql_close(connection);
printw("MySQL session has now closed.\n");
refresh();
return KILL_SIGNAL;
}
MYSQL MYSQL_CONNECT_DB(MYSQL_INFO info)
{
MYSQL *connection;
const char *server = info->IP;
const char *user = info->user;
const char *password = info->pass;
const char *database = info->DB;
connection = mysql_init(NULL);
if(!mysql_real_connect(connection, server, user, password, database, 0,
NULL, 0))
{
//printw("[-] Error: %s\n", mysql_error(connection));
MYSQL *f = KILL_SIGNAL;
refresh();
return f;
}
printw("[+] Connected to MySQL server.\n");
FIRST_TIME_HANDLER(connection);
return connection;
}
void SEND_MYSQL_QUERY(MYSQL conn, const char query)
{
refresh();
if(mysql_query(conn, query))
{
ERROR err;
This isn't all the code that I've done with this project, I think my other code is fine.
```
#include
#include
#include
#include
#include "functions.h"
#include "errh.h"
#include "mysqlerr.h"
void FIRST_TIME_HANDLER(MYSQL *conn)
{
printw("[+] Doing initialization checks...\n");
SEND_MYSQL_QUERY(conn, "create table IF NOT EXISTS user_2(username varchar(25) not null, passwd varchar(25) not null)");
printw("[+] Handler has checked tables.\n");
refresh();
}
void INThandler(int sig)
{
signal(sig, SIG_IGN);
printw("Closing server...\n");
refresh();
sleep(5);
endwin();
exit(0);
}
MYSQL_INFO *CONNECTION_INIT()
{
MYSQL_INFO info = malloc(sizeof(info));
info->IP = "IPADDRGOHERE"; info->user = "Hello";
info->pass = "World"; info->DB = "DBGOHERE";
return info;
}
int EXIT_SERVER(MYSQL *connection)
{
mysql_close(connection);
printw("MySQL session has now closed.\n");
refresh();
return KILL_SIGNAL;
}
MYSQL MYSQL_CONNECT_DB(MYSQL_INFO info)
{
MYSQL *connection;
const char *server = info->IP;
const char *user = info->user;
const char *password = info->pass;
const char *database = info->DB;
connection = mysql_init(NULL);
if(!mysql_real_connect(connection, server, user, password, database, 0,
NULL, 0))
{
//printw("[-] Error: %s\n", mysql_error(connection));
MYSQL *f = KILL_SIGNAL;
refresh();
return f;
}
printw("[+] Connected to MySQL server.\n");
FIRST_TIME_HANDLER(connection);
return connection;
}
void SEND_MYSQL_QUERY(MYSQL conn, const char query)
{
refresh();
if(mysql_query(conn, query))
{
ERROR err;
Solution
-
In C the naming conventions typically are
-
In
-
In
-
There is no reason to do the assignment and compare in one statement. Splitting it up makes the if statement less crammed:
-
I assume
-
There is this magic
In C the naming conventions typically are
lower_case_with_underscore for function names and types (although for types some people also use PascalCase). UPPERCASE is only really used for macro definitions.-
In
CONNECTION_INIT you have a bunch of hard-coded settings. These should come from a config file or via command line arguments.-
In
init_connection you write:if((conn = MYSQL_CONNECT_DB(info)) == (MYSQL *) 0)- There is no reason to cast in order to compare it to
NULL, simply== NULLworks and is more idiomatic.
-
There is no reason to do the assignment and compare in one statement. Splitting it up makes the if statement less crammed:
conn = MYSQL_CONNECT_DB(info);
if (conn == NULL)
{
...-
I assume
printw is some kind of way to print debug/info/tracing messages. If so then this is not a good name for the function. Better would log_info or similar (depending on the exact purpose).-
There is this magic
refresh method called everywhere but it doesn't seem to follow any particular pattern. Seems suspicious to me.Code Snippets
if((conn = MYSQL_CONNECT_DB(info)) == (MYSQL *) 0)conn = MYSQL_CONNECT_DB(info);
if (conn == NULL)
{
...Context
StackExchange Code Review Q#39122, answer score: 4
Revisions (0)
No revisions yet.