9 ms·

Flags Are a Code Smell (2014)

• deleted 4y ago
  [deleted]
• dvt 4y ago
  Flags are not code smells and this article doesn't convince me otherwise. It's basically saying that they can be misused. Okay. Everything (from inheritance, to polymorphism, to functional programming) can be misused, so that's not really an argument against flags themselves. Flags & bitmasks (pun intended) are probably some of the most long-lived patterns in software engineering, so the argument would have to be particularly compelling, which it isn't.
  • enraged_camel 4y ago
    You should read the article more carefully, specifically the second paragraph. Of course it is true that anything can be misused. What makes something a smell, according to the author, is that it is likely to be used incorrectly, perhaps because the distinction between correct and incorrect usage is not immediately obvious. I think the argument has merit and the example supports it sufficiently well.
    • dvt 4y ago
      The article describes an implementation detail: it's still a flag and can still be misused, but the article cleverly avoids discussing how. Component/entity-based engines (which is basically what the proposed solution is) have been around for like 20+ years. Here's me asking about it over a decade ago[1]. [1] https://stackoverflow.com/questions/4941953/the-composite-pattern-entity-system-and-traditional-oop https://stackoverflow.com/questions/4941953/the-composite-pa...
  • uncomputation 4y ago
    You don’t think the ability for a set of mutually exclusive flags to introduce illegal states (a disabled, yet visible object) into a program is a code smell? I think the array approach is far more elegant, if implemented properly (concurrently) and efficiently. More generally, I usually take boolean flags as an indication something may be able to be factored out. Rather than a sort function with a flag alternating between LEQ or GEQ (ie sort(boolean ascending)) , I would prefer separate sort_ascending or sort_descending for example.
    • dvt 4y ago
      > disabled, yet visible object These are bad flags and should not be mutually exclusive. "Disabled" usually applies to the physics system, and "visible" applies to the rendering system. You can certainly have something that renders but does not interact with the physics system (e.g. particle effects). It's like throwing out all operator overloading because somebody decided to make "+" subtract instead of add.
    • ummonk 4y ago
      No that’s not really a code smell for me. It’s really normal for data types not to prohibit illegal states, and that isn’t an issue specific to having multiple Boolean flags. It’s something that a more powerful type system can help solve though (specifically, discriminated union types do this).
    • LudwigNagasena 4y ago
      I don’t see (no pun intended) why disabled yet visible should be considered an illegal state. Disabled is a higher-order concept. You can’t know whether an object should be visible or not after you’ve enabled it without storing that information somehow.
  • Mr_P 4y ago
    It's not so much the existence of the flag, itself, but rather using an if-statement at the deepest-level of the call stack to conditionally modify behavior. This talk gives a great overview of why boolean flags (rather, if-statements) can be a code smell: https://www.youtube.com/watch?v=4F72VULWFvc https://www.youtube.com/watch?v=4F72VULWFvc OP's blogpost advocates for data-oriented design (e.g. Entity Component Systems) as a mechanism for avoiding this, whereas the talk I've linked advocates for OOP. Both mechanisms are equally valid (imho) and are inline with widely-adopted industry practices for software architecture.
    • gentleman11 4y ago
      I second that video: I’ve watched it several times over the years. I still use flags and conditionals, but less of them these days, and it’s made my life a lot easier
  • astrobe_ 4y ago
    Flags are misused if their meaning is entangled, e.g. flag A is meaningless when flag B is set. In this case, you don't need flags but states, as TFA points out (with far too many words, IMO); in C lingo, when this happens you should switch from booleans to enumerations - because programmers worth their salt won't have entangled flags in something designed from scratch, it is more something that can happen when you add features.
    • adhesive_wombat 4y ago
      They also crop up all the time in embedded code, because flags are how lots of hardware works. But, yes, it's usually a different smell in that context because you just know someone is hot-gluing the exact bitmask for a thing into some hardware-agnostic controller code without writing a proper interface that translates from an internal representation (say, an enum) to actual physical bits.
• jef_leppard 4y ago
  If you find yourself passing in flags to change behavior, it sounds like you should just have two functions. For other cases, an enum might serve your needs better as it gives you more options as your codebase grows. That said, I wouldn't call boolean flags a _smell_. Seems like throwing out the baby with the bath water if you ask me.
  • amelius 4y ago
    Yeah, it's either flags or duplicating code. Both are smelly, so pick your poison.
    • jackblemming 4y ago
      No? Both separate functions can call a 3rd shared function if they want. You can also restructure the code with dependency injection. There's plenty of options that don't require duplication
      • jef_leppard 4y ago
        Yep. You said it better than I could. There are a lot better ways to do control flow than passing in a flag. Inversion of control being another great example.
      • amelius 4y ago
        Alas, in reality the code controlled by the flag is often mixed with other code and separating the two is often difficult to do right, e.g. requiring to pass a lot of data hence and forth.
      • gentleman11 4y ago
        That indirection can end up a bit fractal. Sometimes the duplication is less bad than the readability reduction. You have to trust your intuition at those times
  • tbrownaw 4y ago
    Duplication is for when future changes are independent. "This client wants their reports customized" maybe. Flags are for when future changes affect both options. "On top of the usual functionality, we can also optionally handle this other related thing" or so. Or --dry-run handling.
• pylua 4y ago
  I'm not sure I agree with this article. I have been looking at the source code of some open source games and they all make use of a mix of flags and some sort of polymorphism. At some level, when you get into a low enough level class, there is most certainly a ton of if, else, and flags. Not everything can be normalized elegantly into polymorphism, because there are too many combinations. I wouldn't describe these open source games as having code smell, but rather just being very complex. Using flags seems to be a very effective way of encapsulating the complexity. I have not looked at a popular open source game that has really used the approach described in the article. Does anyone have any references for that?
  • mostlylurks 4y ago
    Games are a bit of an atypical case, since the overhead of runtime polymorphism (allocation, virtual function calls) motivate many to avoid runtime polymorphism due to the importance of performance in games, regardless of if that motivation is justified or not. Thus, polymorphism might still have been the most elegant approach, but its use avoided due to non-stylistic factors.
    • pylua 4y ago
      I feel like this is true, but just anecdotally. Is there any good literature on the cost of if/else vs polymorphism? Does branch prediction really work that well in practice vs polymorphism? I figure the approach in the article would potentially have bad cache locality, but I am unsure of how it would perform.
      • tbrownaw 4y ago
        Function pointers / vtables / polymorphism are slow, because they're indirect jumps (harder to predict). Function calls are slow, because there's work to do to save / restore state on the stack. These don't matter much for big heavy functions, but tiny ones can seriously benefit from being inlined. I have heard elsewhere that "game objects are just an index into a bunch of arrays for all the object properties" is fairly common practice, but well I don't write games myself.
• jamesgreenleaf 4y ago
  I'm becoming convinced that using the phrase "code smell" might as well be a "smell" itself. Maybe it's just me, but I've only heard the phrase used as a rhetorical mask for a developer to express some individual preference as though it were an objective fact.
  • HPsquared 4y ago
    More taste than smell, then.
    • jamesgreenleaf 4y ago
      Now you're making sense.
    • sn41 4y ago
      I see you haven't lost your touch.
  • pylua 4y ago
    A lot of IDE plugins will tag code smell as things that may potentially be a bug in your code. I like that more objective use of the term.
  • mostlylurks 4y ago
    Those preferences come from experience. If experience tells you that code written in a certain manner consistently causes certain categories of issues later down the road, it's perfectly justified to be wary of such code.
  • robonerd 4y ago
    When somebody tells you about code smell, ask them to describe the smell. If they can describe it, then you can weigh the merit of their arguments. If 'describe the smell' stumps them, brush their remark aside until they can come back with something more substantial.
    • Brian_K_White 4y ago
• deleted 4y ago
  [deleted]
• FpUser 4y ago
  Article about nothing. Take feature A and declare it code smell and the say that in many cases it is actually not. In programming any single feature / concept is like that. Good for one case, bad for the other.
• aranchelk 4y ago
  > Ok, so far we have three flags which means we have 23 = 8 possible combinations of flags, but do we really have eight separate legal states? No! It’s a good argument for a custom sum type in place of the booleans.
  • gentleman11 4y ago
    Could you elaborate?
    • aranchelk 4y ago
      Sum types let you define a type with a set of fixed possible values (a.k.a. inhabitants). You can define Boolean as a sum type: data Boolean = True | False The pipe character would be read as “or”. Boolean has two inhabitants. True being one, False being another one. 1 + 1 Alternatively, when you have a data structure like the example suggested by the author, it’s a product type with 3 fields, with Booleans inhabiting each (isDynamic, isVisible, and isEnabled), 2 * 2 * 2, 8 inhabitants. The argued criticism of this encoding is that when isEnabled is false, no valid values exist for isDynamic or isVisible. They have no meaning for a disabled object. My earlier definition for Boolean looks like an enum in some languages, but sum types can be more powerful if you’re allowed to define inhabitants with different shapes (curly braces denoting a record with named fields, “::” read as “has type”): data GameObjectStatus = Disabled | EnabledWith {isVisible :: Boolean, isDynamic :: Boolean} Now instead of having 8 inhabitants, you have 5 (a sum of 1 and 4) and they each have a valid conceptual meaning. Languages with algebraic data types (sums and products) like Haskell (and IMO even better PureScript) let you easily define types that have only valid states.
• scollet 4y ago
  Pre-procs are knocking at the door... and it's raining out.
• karaterobot 4y ago
  One of the things I disliked the most about software development was when somebody wrote an article like this, and then somebody on my team would be like "see? you can't use flags, it's a code smell. There's an article about it, and it got on Hackernews and everything. We don't want our code to smell. Flags considered harmful. Will not approve any PRs with flags!" Article Driven Development. People are allowed to have their opinions, but sheesh. Just say "here is a problem you can run into with flags, better be careful!"
  • kevinmgranger 4y ago
    People realized "considered harmful" was too hyperbolic, so they moved to "smell". But then it gets treated the same way, because who wants to be smelly?
    • colejohnson66 4y ago
      “‘Code smells’ Considered Harmful”
      • djbusby 4y ago
        Somewhere I saw a "considered harmful, considered harmful" article. Full meta.
  • deleted 4y ago
    [deleted]
  • trzy 4y ago
    There was someone on my last team who loved using this term in pointless nitpicking critiques. We were a prototyping team. One minute he’d wax philosophical about how prototyping should be done simple and fast, using throwaway code and cobbling together any preexisting bits we can find to move fast, and the next he’d claim that the name “NetworkManager” for the net code client component was “code smell”. Very eye roll inducing.
    • coinbasetwwa 4y ago
• deleted 4y ago
  [deleted]
• jimjimjim 4y ago
  flags are fine. If they reduce complexity. Is it easier to add a comment about what the flags do for the next dev than describe the other structures to avoid the flags? You won't the last dev to touch that code. Write the simplest thing possible.
• derefr 4y ago
  A more specific (and so hopefully less arbitrary) claim for you all: In CRUD apps, database tables with lots of boolean columns are a data-architecture smell. In my experience, the model encoded by such a record is almost always a — perhaps implicit — finite state machine of some kind. Usually, in CRUD stuff, this is a "lifecycle state" machine. (For example, the BlogComment lifecycle: draft → awaiting moderation → visible → deleted.) But, because the people who create and evolve data schemas aren't usually familiar with FSMs, there's often no explicit "lifecycle state" enum-typed column in the DB record. Instead, you get this pile of boolean columns, where each column represents the answer to a predicate that would take the model's lifecycle state as its input. Think e.g. `can_login?(User)` — which really just boils down to `can_login?(UserLifecycleState)`. If you add the lifecycle-state column, then all the predicate-answer columns can go away. As long as you formally define+document the meaning of each lifecycle state, you can then statelessly recompute all those predicate-answers any time you like, on any layer of the stack you like. And now those predicate-answers will never be able to end up in an incoherent state, because they're never persisted; only the lifecycle-state itself is. Of course, if you actually go all the way and make your model an FSM on the business layer as well, then you'll also get as a free benefit, the ability to validate lifecycle-state transitions — e.g. "Articles should only be able to enter the Published state from the Reviewed state."
  • smegsicle 4y ago
    yeah nice, way more coherent than that article seemed
  • onion2k 4y ago
    A flag is just a memoized answer to the predicate though.
    • derefr 4y ago
      If you also explicitly store the FSM state itself, then sure, that's true. But in my experience, in almost all of these cases, the flags are stored in lieu of an FSM state; with the current FSM state implicit as a constraint-solution over the current set of predicate-answers. Which allows for incoherent states, because no FSM is actually being modelled in the business logic — even though the underlying business process the code is supposed to be modelling is inherently one involving state transitions.
• sn41 4y ago
  Flags are a universally understood programming idiom. It is not the best way, but it is well-understood. And that counts for a lot in programming practice.
• ChrisMarshallNY 4y ago
  I don’t like using them, but, sometimes, they are required. That pretty much sums up all kinds of “smelly” practices. In my experience, “God said it, I believe it, and that settles it” type pronouncements end up looking fairly silly. We do what needs doing, to achieve our ends. At least, that’s what most experienced ship programmers do. What I prefer (I program in Swift), are computed properties that examine the current state, and synthesize a report on demand. Can’t always do that, though. Things like performance and threading can be factors. I just ran into something like that, tonight. Not sure how I’ll solve it. I’ll sleep on it.
• mbrodersen 4y ago
  Nope. Not at all. For example, a friend of mine is working on an enterprise system used by a number of very large international companies. Every company demands slight variations to everything. From UI look/flow to how low level business logic is done. Flags are perfect for this. The key of course is to make sure that the flags are used correctly. By (for example) having a validation step at startup making sure the flags are configured correctly. Or (even better) using types to make it impossible to configure it incorrectly.
  • tgv 4y ago
    So to me that sounds less than perfect. Enterprise systems tend to be large, so it can be hard to keep track of the flags, and splitting flags can become a source of bugs. A simpler solution would be a (global, singleton) class that offers an interface to customisation. Of course, that might not be possible in an existing system, but it offers advantages over flags.
    • deleted 4y ago
      [deleted]
• mbrodersen 4y ago
  I am sick and tired of “X is a code smell” articles. Everything in software development depends on context. What makes perfect sense in context A makes zero sense in context B. If (for example) all you know is front-end JavaScript development then don’t make claims about anything outside of that context. Have you ever only worked in one industry? Then perhaps realise that your experience might be limited to that context. While being completely and utterly wrong in a different context. I have worked in many different contexts in my 25+ years career. AAA game development, high-performance compilers, low-powered embedded systems, typescript/react front-end development, enterprise software applications, mobile apps etc. And the #1 thing I have learned is that nothing is true or false in all contexts. Everything depends on your particular situation and context. So be aware of your limited context and perhaps think about what would no longer be true/false when you change the context.
  • synu 4y ago
    I recall at one point a code smell meant something more like “this may be off, but not necessarily, so you should investigate and be sure in your context.” It was better than “considered harmful”, which was more absolute. That nuance may once again be lost.
    • patrick451 4y ago
      Ehh. All code smells. Especially if it was written by somebody else. Or yourself more than six months ago. Get some nose plugs and move on with life.
      • eurasiantiger 4y ago
        If code doesn’t smell like anything, it probably doesn’t do any heavy lifting.
        • rightbyte 4y ago
          Hiding complexity by smearing it out over abstractions and more loc:s is the name of the game.
• darzu 4y ago
  Putting stock in code smells is a process smell
• tbrownaw 4y ago
  Most of the individual boolean flags in the code I work with are retrieved on-demand from a feature-flag store. Well, aside from the dryRun flag that a lot of utility things take. But this actually seems to be talking about having object state represented internally as a pile of flags when something more semantically meaningful is available.
• 29athrowaway 4y ago
  There was an opportunity here to speak about finite state machines.
• anfilt 4y ago
  A bit hyperbolic calling code smell. They even mention they have their uses. I think the main point of this post is be careful with state explosion since each flag grows the number of states exponentially and likely not all of then are legal or make sense. The other point is grouping similar things in memory is good for performance. Less failed branch predictions from checking flags and better cache coherence. Thats not as a click bite-y of a title though. Although the above is sound advice.
• lenkite 4y ago
  If the code ain't smelling, it ain't working!
• shrimpx 4y ago
  The article's most interesting point is that the vast majority of possible program (or object) states tend to be invalid. For example, `Person(isWalking=true, isAlive=false)` is nonsense, because there's an unexpressed constraint that `isWalking==true ==> isAlive==true`. Though the author veers in another direction, I think his core point is related to this discussion: Applying “make invalid states unrepresentable” https://news.ycombinator.com/item?id=24685772 https://news.ycombinator.com/item?id=24685772
  • aastronaut 4y ago
    Also a great talk from Richard Feldman about "Making Impossible States Impossible": https://www.youtube.com/watch?v=IcgmSRJHu_8 https://www.youtube.com/watch?v=IcgmSRJHu_8
  • merlincorey 4y ago
    > The article's most interesting point is that the vast majority of possible program (or object) states tend to be invalid. For example, `Person(isWalking=true, isAlive=false)` is nonsense, because there's an unexpressed constraint that `isWalking==true ==> isAlive==true`. Are you implying that Undead aren't people? Also, magical Undead states aside, I'm pretty sure it is possible for a Person to be Walking while they are not technically Alive if something else is able to control their motor system as happens in nature with various Wasps, Slime Balls, and more.
    • shrimpx 4y ago
      You make good points. Some of these "illegal" states may be legal from some other perspective, or yet-unforeseen use cases.
    • gentleman11 4y ago
      Bug or feature?
• lloydatkinson 4y ago
  Creating many boolean states leads to an explosion of complexity and states. You can use a finite state machine to replace these states as I wrote here: https://www.lloydatkinson.net/posts/2022/modelling-workflows-with-finite-state-machines-in-dotnet/ https://www.lloydatkinson.net/posts/2022/modelling-workflows...
• awesomedad 4y ago
  I notice that these code smells as discussed are often really a function of the domain the developer is in. Lots of game examples, saw some sql too. What for me is the main reason not to like flags/booleans, is the current hype of MVP. Yeah, sure, a decision cam be yes or no for the current process. But what about, yes, but.... or no, unless.... for later iterations? Also, focussing on current user requirements leads to many booleans. What about, when the process is optimal, is it still a yes/no question? I also notice questions on different levels. In case of visible and static, for instance. One has to do (i guess) with rendering behaviour, while the other has to do with (let me call it) game engine behaviour. Is it then maybe also a case of (re-)defining the data models, as the element you're trying to specify gains depth? Even, possibly, that your definitions have become convoluted? In the game example, a difference between the element as an actor in a game scene as opposed to its representation for a render engine?
• ehutch79 4y ago
  Shit. I guess I should remove the flags from my models. But now how do I tell if a company is a qualified vendor or not? If a user is active and can login. I could have sworn I just read an article about putting code that's not ready behind a feature flag instead of long running git branches... guess that was a fever dream?