54

I guess this is another question about hard coding and best practices. Say I have a list of values, lets say fruit, stored in the database (it needs to be in the database as the table is used for other purposes such as SSRS reports), with an ID:

1 Apple 
2 Banana 
3 Grapes

I may present them to the user, he selects one, it gets stored in his profile as FavouriteFruit and the ID stored in his record in the database.

When it comes to business rules / domain logic, what are the recommendations for assigning logic to specific values. Say if the user has selected Grapes I want to perform some extra task, what's the best way to reference the Grapes value:

// Hard coded name
if (user.FavouriteFruit.Name == "Grapes")

// Hard coded ID
if (user.FavoriteFruit.ID == 3) // Grapes

// Duplicate the list of fruits in an enum
if (user.FavouriteFruit.ID == (int)Fruits.Grapes)

or something else?

Because of course the FavouriteFruit will be used throughout the application, the list may be added to, or edited.

Someone may decide that they want 'Grapes' renamed to 'Grape' and this would of course break the hardcoded string option.

The hardcoded ID isn't completely clear although, as shown you could just add a comment to quickly identify which item it is.

The enum option involves duplicating data from the database which seems wrong as it may get out of sync.

Anyway, thanks in advance for any comments or suggestions.

Kate
  • 643
  • 5
  • 7
  • 1
    Thanks everyone: the suggestions and general advice is really helpful. @RemcoGerlich your idea to separate the concerns of a string used for display purposes and a separate one as a lookup code for more readable code is very good. – Kate Dec 03 '15 at 14:37
  • 1
    I'm going to give @Mike Nakis your preloaded objects idea a go though as this seems like the best of both worlds. – Kate Dec 03 '15 at 14:37
  • 1
    I would suggest a variation on your first solution. Have your table contain a 3rd column for how it will be processed, and you use that field to determine what code to execute. Not a display field, and can be shared between multiple fruits. – Kickstart Dec 03 '15 at 16:55
  • What sort of "extra task" would you want to do? What is so special about 'Grapes'? From a database perspective, we generally avoid coding references like that, preferring instead to add extra columns or tables that allow us to select items based on specific traits (eg. select all the fruits that have eatable flesh rather than select where id in (2, 3)). It seems like you should be leveraging the database to determine your tasks (possibly even in the form of a trigger, rather than extra logic in your application). – cimmanon Dec 03 '15 at 18:29
  • 1
    *The enum option involves duplicating data from the database which seems wrong as it may get out of sync.* I like this, actually. It's like double entry bookkeeping. If both sides of the ledger don't balance you will know something is wrong. It makes changing things more deliberate. – radarbob Dec 04 '15 at 03:02
  • 1
    Hmmm... If there is a 1:1 relationship of ID to a String then this is redundant and having both is pointless. A String can serve as a DB key just as well as an integer. `MyApplication.Grape.ID` is stuttering, so to speak. A "Apple" is not a "Red_Apple" any more than ID of 3 is also 4. So the potential for renaming "Apple" to "Red_Apple" makes no more sense than declaring that 3 is 4 (and maybe even 3). The point of an enum is to abstract away its numeric DNA. So maybe it's time to *really* decouple arbitrary relational DB keys that have literally no meaning in one's business models. – radarbob Dec 04 '15 at 03:36
  • How about a new boolean column in the database called "IsGrape"? – Mark Pattison Dec 04 '15 at 10:42
  • @radarbob I agree. When I started to implement it I realised that having a separate 'code' in the database and having the application use it as the way of identifying fruit, while other tables in the DB did so by the ID was doubling up and also risky as it suggests the application doesn't care if the numeric IDs changed. In the end, I went with the enum suggestion so the key field is the numeric ID as far as both the app and DB is concerned but the code can clearly use my enum description so is readable. This was simpler to implement than the preloaded objects although I will try that too. – Kate Dec 07 '15 at 15:27
  • @cimmanon I'm not keen on the idea of triggers or putting functionality in the database, preferring to keep functionality in the code. This was just a contrived example - the real app isn't about fruit. The functionality in question is that a workflow has stages and at different stages different things happen. If a person is at a particular stage then the options displayed (such as add another stage) are particular to the stage they are on. – Kate Dec 07 '15 at 15:33
  • @simon; I always convert/cast to an enum member up front (potentially throwing an exception) and never use the numeric (or string) value explicitly. When my enum's underlying numbers have meaning I always use the enum member. ... Using enums for sort order `CompareTo()` has this: `if (( this.ServiceType > other.ServiceType ) return 1;` and elsewhere: `if( myServiceType != ServiceType.ACB )..` Oh, there's one more thing... with inherent enum type safety, renaming members is a non issue. – radarbob Dec 07 '15 at 16:39

7 Answers7

35

Avoid strings and magic constants at all costs. They are completely out of the question, they should not even be considered as options. This appears to leave you with only one viable option: identifiers, that is, enums. However, there is also one more option, which in my opinion is the best. Let's call this option "Preloaded Objects". With Preloaded Objects, you can do the following:

if( user.FavouriteFruit.ID == MyApplication.Grape.ID )

What has just happened here is that I have obviously loaded the entire row of Grape into memory, so I have its ID ready to use in comparisons. If you happen to be using Object-Relational Mapping (ORM), it looks even better:

if( user.FavouriteFruit == MyApplication.Grape )

(That's why I call it "Preloaded Objects".)

So, what I do is that during startup I load all of my "enumeration" tables (small tables like days of the week, months of the year, genders, etc.) into the main application domain class. I load them by name, because obviously, MyApplication.Grape must receive the row called "Grape", and I assert that each and every one of them is found. If not, we have a guaranteed run-time error during startup, which is the least malignant of all run-time errors.

Mike Nakis
  • 32,003
  • 7
  • 76
  • 111
  • 17
    I don't disagree with the answer, but I think the imperative to "Avoid strings and magic constants at all costs" disagrees with the rest of the answer, which actually *requires* that you have *at least one place* where magic constants or strings are used in populating your "preloaded objects." That's notable, I think, because there *are* ways of avoiding "strings and magic constants" entirely, though it's usually more obfuscating than it's worth ... – svidgen Dec 03 '15 at 15:24
  • 2
    @svidgen wouldn't you agree that there is a fundamental difference between scattering binding-by-name all over the place vs. binding-by-name only once, to load the contents of a record bearing the same name, and doing so only at startup, where runtime errors are almost as benign as compilation errors? Anyway, ways of avoiding even the slightest binding by name are always interesting, despite the obfuscation that you mentioned, so I would be curious to hear what you have in mind. – Mike Nakis Dec 03 '15 at 15:37
  • Oh, I agree completely. And, given the nature of the OP, I'd only suggest this answer could benefit from changing "at all costs" to "whenever possible and feasible" or something similar. ... If I had more time, for the sake of completeness only, I'd write up an answer that deals with some sort of metaprogramming nonsense ... but, that's not what the OP (or anyone in most cases) probably needs. But, a metaprogamming solution would align more with your first statement as-is. – svidgen Dec 03 '15 at 15:39
  • Do you mean some mechanism of automagically binding enum values to row records with the help of, perhaps, some annotation or something? – Mike Nakis Dec 03 '15 at 15:43
  • No. I'm thinking more of taking the conditional (or whatever it may be) out of the core application and pushing it into the data. One option at the more flexible end, is an embedded scripting language or compiler that *at specific times* uses a context or API provided by the application. At the less flexible end, you actually build the schema in a manner that better expresses the rules. And that doesn't mean you totally eliminate strings from the app, but they become "non-magical." Embed the rules in the schema rather than the data. E.g., no `roles` table, but `isAdmin`, `isMod` fields etc. – svidgen Dec 03 '15 at 16:20
  • And again, I don't intend to pretend that sort of thing is the "right" answer here; but it's *an* answer. And I think *that's* the sort of answer I'd expect to see following your initial statement. – svidgen Dec 03 '15 at 16:22
  • Not saying that the proposed solution is poor but there is no difference between that and declaring a application wide constant for the known ids, or an enum with the known ids. In both those cases, and in the proposed solution, the 'ids known ahead of time' are defined in a single place. So I agree with svidgen that the 'avoid magic constants' statement at the beginning of the answer does not match the proposed solution since it is essentially a variant of a 'magic constant' (if that's the term you want to use) – user469104 Dec 03 '15 at 17:55
  • 1
    @user469104 the difference is that the ids may change, and the application will still load all rows correctly and perform all comparisons correctly. Also, you are free to refactor code and rename rows in whatever way you like, and the only place where you have to go looking for stuff to fix is in the startup of the application, and it tends to be very obvious: `Grape = fetchRow( Fruit.class, NameColumn, "Grape" );` And if you do something incorrectly, an `AssertionError` will let you know. – Mike Nakis Dec 03 '15 at 18:00
  • So, what I am suggesting here is that the fact that the one and only occurence of "Grape" is in the initialization of the identifier `Grape` makes it non-magic. Functionally, it is equivalent to `define constant Grape = "Grape"`. – Mike Nakis Dec 03 '15 at 18:01
  • @Mike Nakis I would suggest that having a dependency on the string "Grape" is a weaker solution than the dependency being on an Id. Typically, the chances of a name or description of something changing is greater than an Id changing. – user469104 Dec 03 '15 at 18:13
  • @user469104 I would argue that it is not, because if the id changes, you are left with no hint as to what has changed, what remains valid, and what is now invalid; you don't know what the ids used to be, and you don't know what they became. If 5 gets removed and therefore 6 gets renumbered to 5 and 7 gets renumbered to 6, you are in real deep trouble. On the other hand, if "Grape" becomes "RedGrape" and "WhiteGrape", you can still make perfect sense of everything. – Mike Nakis Dec 03 '15 at 18:18
  • I would agree that it depends on the stability of the ids. In many systems ids are stable over time and environments while names and descriptions can change. In those scenarios having constants for the ids would be preferred. If the ids can change over time or between environments, having a fast-fail on a name change could be beneficial though if names can change for valid reasons it would be a double-edged sword. I.e. if your app fails to run from a valid change to a name or description. – user469104 Dec 03 '15 at 18:33
  • While it is platform (and version) specific, .net now has the NameOf which can be used for this -- use the name of the property to fetch the corresponding row from the database and compare it to the row/value in question. – jmoreno Dec 04 '15 at 05:52
  • Haven't you just added a magic string "Grape" that happens to be hidden by some ORM syntactic sugar? – grahamparks Dec 13 '15 at 22:40
  • 1
    @grahamparks not any more than an `enum` would have been a magic string. The point is ***concentration of all binding-by-name in just one place***, ***validation of it all during startup***, and ***type safety***. – Mike Nakis Dec 13 '15 at 22:46
8

Checking against the string is the most readable, but it does double duty: it's used both as an identifier and a description (that may change for unrelated reasons).

I usually split both duties into separate fields:

id  code    description
 1  grape   Grapes
 2  apple   Apple

Where description may change (but not "Grapes" to "Banana"), but the code is not allowed to change, ever.

Although this is mostly because our ids are almost always auto-generated, and thus not a good fit. If you can choose ids freely, maybe you can guarantee that they are always correct and use those.

Also, how often does someone really edit "Grapes" to "Grape"? Maybe none of this is necessary.

RemcoGerlich
  • 3,280
  • 18
  • 22
  • 8
    I don't think yet more redundancy is the answer... – Robbie Dee Dec 03 '15 at 14:01
  • 5
    I have considered this option too, and I have tried it, but this is what ended up happening: at some point, "apple" had to be differentiated into "green_apple" and "red_apple". But because "apple" was already used in a myriad of places in the code, I could not rename it, so I had to have "apple" and "green_apple". And as a result, the Sheldon in me prevented me from sleeping for several nights until I went in there and refactored everything to "Preloaded Objects". (see my answer.) – Mike Nakis Dec 03 '15 at 15:04
  • 1
    I definitely like your Preloaded Objects, but if your "apple" is differentiated, don't you have to go over everything anyway, whatever method you choose? – RemcoGerlich Dec 03 '15 at 15:14
  • You might even have a separate table for the description name, in support of internationalization. – Erik Eidt Dec 03 '15 at 17:16
  • @RemcoGerlich no, I only go to one place: there where it says `Apple = fetch( Fruit.class, NameColumn, "Apple" )` I refactor -> rename `Apple` to `GreenApple` and I change "Apple" to "GreenApple". – Mike Nakis Dec 04 '15 at 13:55
  • 1
    @MikeNakis and the Refactoring is essentially a Search&Replace over your whole Codebase replacing Fruit.Apple with Fruit.GreenApple. If I use Hardcoded String values I would do a Search & Replace over the whole Codebase to replace "apple" with "green_apple" which is about the same thing. -- Refactoring does just feel better, because the IDE is doing the Replacing. – Falco Dec 04 '15 at 15:51
  • This approach is a lot less hassle, if the Values can change at runtime and different Modules all over the application will check for different IDs at some place or another. If a new value is added and Module X wants to use it, you just have to redeploy module X. If you use a shared Base-Class which holds Enums for all Values, you have to Edit the Base-Class and redeploy all applications... – Falco Dec 04 '15 at 15:56
  • @MikeNakis: Generally if an "apple" value is differentiated into "green apple" and "red apple", then _some_ instances of `user.favouriteFruit == MyApplication.Apple` would change to GreenApple, and some to RedApple. The case where they all change to the same kind of apple is hardly differentiation. – RemcoGerlich Dec 04 '15 at 17:38
5

Storing them in both places (in a table and in an ENUM) is not that bad. The reasoning is the following:

By storing them in a database table, we can enforce referential integrity in the database via foreign keys. So when you associate a person or whatever entity to a fruit it is only a fruit that exists in the database table.

Storing them as an ENUM also makes sense because we can write code without magic strings and it makes the code more readable. Yes, they do have to keep in sync, but really how hard would it be to add a line to the ENUM and a new insert statement to the database.

One thing, once an ENUM is defined, do not change it's value. For example, if you had:

  • Apple
  • Grape

DO NOT rename Grape to Grapes. Simply add a new ENUM.

  • Apple
  • Grape
  • Grapes

If you need to migrate data, then apply an update to move all the Grape to Grapes.

Jon Raynor
  • 10,905
  • 29
  • 47
  • As a further step I've worked in shops where metadata values have a delete flag in the table to indicate that they shouldn't be used (they've either been deprecated or a newer version exists). – Robbie Dee Dec 04 '15 at 10:49
4

What you are expecting here is the programming logic to be automatically adaptable to changing data. Simple static options like Enum don't work here because you cannot feasibly add extra enums in the runtime.

A few patterns I have seen:

  • Enums + default to protect against a brand new database entry ruining your program's day.
  • Encoding of actions to be done (biz logic) in the database itself. In many cases this is very much possible because many logics get reused. The implementation of the logic should be in the program.
  • Extra attributes/columns in the database to mark the brand new value as 'to-be-ignored' in the program until program is deployed right.
  • Fail fast mechanisms around the code path that loads/reloads the values from the database. (If the corresponding action is not in the program AND it is not marked to be ignored, don't take the refresh).

In general I like data being complete in terms of referring to actions that it implies - even though the actions themselves might be implemented elsewhere. Any code that is determining actions independent of the data has just sharded your data representation which most probably will diverge and lead to bugs.

Subu Sankara Subramanian
  • 2,438
  • 15
  • 23
2

You're right to ask this question, it's a nice question actually as you're attempting to defend against the evaluation of inaccurate conditions.

That said, the evaluation (your if conditions) don't necessarily need to be the focus of how you get around it. Instead, pay attention to the way you propagate the changes that would cause an 'out of sync' issue.

String Approach

If you must use strings, why not expose the functionality of changing the list through the UI? Design the system so that upon changing Grape to Grapes, for example, you update all of the records currently referencing Grape.

ID Approach

I'd always prefer to reference an ID, despite the compromise of some readability. The list may be added to can again be something you would be notified of if you exposed such a UI feature. If you're concerned with the reordering of items changing the id, propagate such a change to all dependent records again. Similarly to above. Another option (following proper normalisation convention, would be to have a your enum/id column - and reference a more detailed FruitDetail table, which has an 'Order' column you can lookup).

Either way, you can see I'm suggesting to control the amendment or updating of your list. Whether you do this through the use of an ORM or some other data access is determined by the specifics of your technology. What you're, essentially, doing is requiring that people that away from the DB for such changes - which I think is fine. Most main CRMs will make the same requirement.

JᴀʏMᴇᴇ
  • 2,361
  • 4
  • 22
  • 25
  • 1
    _In the database_, the numerical id is being stored for child records, specifically to avoid that problem. This question is about how to interface to a programming language. – Clockwork-Muse Dec 03 '15 at 13:29
  • 1
    @Clockwork-Muse - to avoid which problem? That doesn't make sense. – JᴀʏMᴇᴇ Dec 03 '15 at 15:57
  • I use the ID approach quite a bit, but the ID is locked down and can't change. The attached string of course can because people often like to rename things "lorry" becomes "truck", etc, while the thing itself (represented by ID) does not change. – Brian Knoblauch Dec 03 '15 at 21:19
  • If you go with the ID approach, how do you deal with development vs production databases? With auto-incremented IDs, adding items to both DBs in different order will result in different IDs. – Protector one Jul 17 '19 at 11:21
  • Doesn't have to be auto increment though? It shouldn't be in this case, especially if it's the underlying enum's integer value we're using. – JᴀʏMᴇᴇ Jul 18 '19 at 08:34
1

A very common problem. While duplicating the data client side may seem to violate DRY principles, it is really due to the difference in paradigm between the layers.

Having the enum (or whatever) out of step with the database isn't actually that uncommon either. You may have pushed out another value to a metadata table to support a new reports feature which isn't yet used in the client side code.

It sometimes happens the other way too. A new enum value gets added to the client side but the DB update can't happen until such time as the DBA can apply the changes.

Robbie Dee
  • 9,717
  • 2
  • 23
  • 53
  • Yes, you have described the problem. What is your solution? – Protector one Jul 17 '19 at 11:22
  • 1
    @Protectorone You assume there *is* a silver bullet solution which is an erroneous assumption in my experience. The best you can hope for is that some business entity owns the problem domain so at least you can see which side is lagging - client side or database side. Banking and finance are typically very efficient in this regard with the retail sector being noticeably less so... – Robbie Dee Jul 18 '19 at 00:04
1

Assuming that we're talking about what is essentially a static lookup then the third option - the enum - is basically the only sane choice. Its what you'd do if the database wasn't involved so it makes sense.

The question then becomes the one about how to keep enums and static / lookup tables in the database in sync and unfortunately that's not a problem I yet have a complete answer to.

By choice I do all my schema maintenance in code and therefore I can keep a relationship between a build of the application and an expected schema version so it is straightforward to keep the lookup and the enum in sync but its something that one has to remember to do. It would be better if it were more automated (and also an automated integration test to ensure that the enums and the lookups match would help) but that's not something I've ever implemented.

Murph
  • 7,813
  • 1
  • 28
  • 41
  • 1
    I don't believe these are just static lookups, otherwise they could just be pulled from the database and consumed as-is. The problem as I understand it is when business logic is to be applied depending on the lookup value used. But, that aside, yes - enums are generally employed for this purpose. – Robbie Dee Dec 03 '15 at 13:38
  • Ok, I need a better term "for static lookup" the context you describe is what I meant :) The key is "static" - these are values that don't change the issue is adding new values and changing the "label" (but not the intent) for existing values. – Murph Dec 04 '15 at 10:46