1

How to write Readable Comment when we have to deal with IDs?

I will explain this with example.

+---------------+-----------------+
| permission_id | permission_name |
+---------------+-----------------+
|             1 | Add Category    |
|             2 | Delete Category |
|             3 | Add Product     |
+---------------+-----------------+

And there is a table I maintain which will store which permissions has each user. When logging a user I saved all permissions to Session array.

So session array like,

$_SESSION['permissions'] = array(1,2); // This user has Add Category & Delete Category permissions.

My code is like,

function delete_category(){
    if(in_array('2')){

        //Code
    }
    else{
        // Sorry You have no permission to do this task.
    }
}

I have problem in line 2, which is if(in_array('2')) because even me and other has no idea what is this 2. I mean we have to check the database to find it.

It is is not readable code, I have this problem not just in this case. There are many places I have this problem.

How to write readable code when this type of situation?

  • 1
    Possible duplicate of ["Comments are a code smell"](https://softwareengineering.stackexchange.com/questions/1/comments-are-a-code-smell) – gnat Aug 11 '17 at 06:09
  • 5
    Look up "magic numbers" and "symbolic constants". One is an anti-pattern, the other is the solution. – Kilian Foth Aug 11 '17 at 06:09
  • @KilianFoth I did not get what you said :-( – I am the Most Stupid Person Aug 11 '17 at 06:34
  • 5
    @DonkeyKing You should never use unexplained ('magic') literal numbers in your code. Instead you should define `DELETE_PERMISSION=2;` (that's a symbolic constant) and then write `if(in_array(DELETE_PERMISSION))` instead. – Kilian Foth Aug 11 '17 at 06:36

1 Answers1

13

You should not use integer id's directly in logic. You should define constants (or enums or whatever is appropriate for your language) corresponding to the id's and then use the names. E.g.

Permissions.AddCategory = 1
Permissions.DeleteCategory = 2
Permissions.AddProduct = 4

...

if(in_array(Permissions.DeleteCategory)){

...

This much easier to read, write and understand. You don't even need the comments then, since the code itself explains what happens.

JacquesB
  • 57,310
  • 21
  • 127
  • 176