1

I have these tables: contacts, tags, contact_tags.

(contact_tags has columns contact_id and tag_id.)

Administrators can manually create new tags via an internal website.

Administrators can also associate a tag to a contact (i.e. create a new contact_tag).

Non-admin visitor behaviors can also cause the creation of a contact_tag.

Certain tags are really important, and when one of these important tags is being associated with a contact (i.e. its tag_id is in the contact_tag being created either manually by an admin or via a visitor behavior), certain functions need to run.

To achieve this, there are various tag IDs hard-coded in my code. This smells bad, but I haven't figured out a better approach, so I'm wondering what the best practice is.

For example, my code might say:

const TAG_DEPOSIT_PAID = 67;
//(and many other constants of IDs specified here too)

and then somewhere in ContactTagAddedEventListener:

if($tag->id == Tag::TAG_DEPOSIT_PAID){
    //do stuff
}

How should can I improve my architecture?

(By the way, for testing purposes, my database seeder seeds a testing database with all the "important" tags referenced by these different functions.)

P.S. My understanding is that enums are almost always bad and that tables like my tags table are generally better, but I'm clearly missing some other principles because my approach doesn't feel clean.

3

Yes, this smells and proves to be very inflexible.

If it would just be about important vs. less important tags, I'd just mark those in the tags table with a property is_important.

But your code example suggests that there may also some special, application specific behavior behind it. This creates an issue since the application behavior is tightly coupled to some data values.

The usual way to handle this is to make a difference between the tag-id that is created by the admin (or partly automatically), and the special codes that are internal. Example:

tag-id    tag-name                           internal-code
100       deposit paid per credit card       67
101       deposit paid per vire transfer     67
102       new subscriber 
103       subscription to be paid            66

This lets you the freedom to describe several variants of a tag that would share the same behavior. It lets you regorganize the tags as you want. It would make in the data very clear what tags are linked to internal behavior and the ones that are not.

  • I appreciate your answer. In my case, I would never need multiple different tag records to share a same "internal-code", so I can't tell what your approach would improve. – Ryan Sep 9 at 17:34
  • Or maybe I'd make internal-code a nullable varchar with "unique" index, and it would smell a little less bad because then my code would do lookups of tags using that human-readable string instead of an ID, and (as you said) someone looking at the table could easily see which tags were involved in logic within the code. – Ryan Sep 9 at 17:44
  • 1
    @Ryan it would at least highlight the dependency between the tags that are used in the programme logic (internal code) and the other ones (no internal value). You can then easily get a list of tags with internal code to cross check that nothing was forgotten. In addition, the internal value allows you to have several different database instances (e.g. dev, test and production or even different instances) with the certainty of ensuring that internal codes are aligned with code expectations, whatever the id is (especially if the id is determined automatically) – Christophe Sep 9 at 18:32
  • 1
    I'm accepting this answer because I added a 40-char column called "slug" ($table->string('slug', 40)->nullable()) with a unique index. My code now has a class of constants where the slugs are hard-coded (instead of IDs). When I look at the database, I can tell which tags are dependencies of the code (by which ones have slugs). It still doesn't feel fantastic, so I wouldn't be surprised if I someday learn of a better pattern, but it's an improvement from my earlier approach. – Ryan Sep 12 at 15:37
  • @Ryan thanks for this feedback ! – Christophe Sep 12 at 16:22
1

Yes it does smell just a little :)

The first improvement you could make is to encapsulate the id comparisons inside the Tag class by providing boolean methods like tag->isDepositPaid() and customer->hasPaidDeposit(). There will be a tiny performance hit - probably impossible to measure as database operations are likely going to be the performance limiting bottleneck for your application. With careful coding, the constants will become private to the Tag class.

The second improvement is to remove the dependency on the id field value. What you are trying to do is map an enum to a database table. Try searching this forum's questions on database enum. There are some well considered answers.

My preferences is to use a short string (say VARCHAR(20)) as the primary key of the Tag table. This will upset the database purists, but has a advantages:

  • It avoids having a numeric primary key plus a unique label field that is also a type of primary key. In my view, having two keys violates the DRY principle.
  • The field is human readable. This is a huge advantage when debugging. No need to remember that 42 is the meaning of life.
  • If you serialize the object to JSON or XML the field is automatically human readable in the serialized data. This is a huge advantage in large enterprise systems where data is passed to and from middleware layers.

If you stick with using id comparisons, make sure the Tag table does NOT have an auto incrementing primary key. These can get out of step between instances of the database. For example, an Oracle database running on a cluster doles out ranges of keys to the cluster members. There is no guarantee that the values used will be consecutive.

  • Thanks for your answer. In PHP, the only place that has the ID number is where the constant is defined (and of course in the table data). Everywhere else is human-readable, like Tag::TAG_DEPOSIT_PAID. So that hasn't been a problem. And my database seeder takes care of the problem that would be caused by auto-incrementing. – Ryan Sep 9 at 17:37

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.