CODEWITHSUNDEEP
phpmysqloopdatabase-schema
paul
paul

Reputation: 741

Avoid hard coding in mysql query

I have a table of ban reasons:

id | short_name | description
 1      virus     Virus detected in file
 2      spam      Spammy file
 3      illegal   Illegal content

When I ban a file for being a virus, in my code I do this:

$file -> banVirus();

Which inserts the file id and ban reason into a table:

"INSERT INTO 'banned_files' VALUES (61234, 1)"

My question is; is it a problem that I have hard-coded the value 1?, to indicate a spam file.

Should I use defines in my config like define ('SPAM', 1), so i can replace 1 with a define? Or does it not matter at all?

Upvotes: 2

Views: 950

Answers (2)

James Cronen
James Cronen

Reputation: 5763

Since you have a fixed (and small) number of parameters, I'd be tempted to make the IDs an enum in your code and not even include them as a separate database table at all.

Think about something like gender -- which has two (or more) options, both fixed. (We won't be adding multiple new genders anytime soon.) I guarantee most registration systems' don't have a GENDER table with two entries in it.

So, table banned_files would be something like this:

id      | reason
--------+------------
12345   | 1
67890   | 2

and your code would contain enums as necessary:

enum BanReason {
    Virus = 1,
    Spam = 2,
    Illegal = 3
}

(please convert to PHP; I'm a C# developer!)

In PHP:

$aBanReason = array(
    'Virus' => 1,
    'Spam' => 2,
    'Illegal' => 3
);

Upvotes: 1

deceze
deceze

Reputation: 522382

If the id is an auto incrementing field, then it is a very big problem! Since the ids are automatically generated, it's hard to guarantee their stability; i.e. they may change.

If the id is something you manually assigned, it's not such a big problem, but it's bad practice. Because magic numbers easily lead to confusion and mistakes. Who knows what "1" means when reading your code?

So either way, you'd be better off to assign a stable, readable id to each case.

I agree with @Tenner that it also hardly makes sense to have a table for this static, unchanging data to begin with. Your banned_files table should have a column like this:

reason ENUM('virus', 'spam', 'illegal') NOT NULL

You need nothing more in your database. When outputting this for the user, you can add a readable reason with a simple array through your PHP code.

Upvotes: 2

Related Questions