HiveBrain v1.2.0
Get Started
← Back to all entries
snippetcMinor

Is my MySQL library wrapper neat? How can I do better?

Submitted by: @import:stackexchange-codereview··
0
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;

Solution

-
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 == NULL works 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.