prompt-injection icon indicating copy to clipboard operation
prompt-injection copied to clipboard

Convert basic enums to simple objects

Open pmarsh-scottlogic opened this issue 2 years ago • 9 comments

I see typescript enums being used in a few places in the code, e.g. DEFENCE_TYPES.

There is often a better way to achieve this using the power of types. Refer to https://www.typescriptlang.org/docs/handbook/enums.html#objects-vs-enums

Why are enums confusing? Because it is not always clear when you are referring to the enum name, and when to the (numeric or string) value. Numeric enums are particularly problematic as name and value are different, whereas String-valued enums seem a bit un-DRY when, as is often the case, the name is exactly the same as the value.

This is a placeholder issue for identifying all enums, and assessing which would be better represented using type narrowing and standard javascript objects. The first of these can then be used to better describe the issue, and a possible alternative. A likely candidate is #355.

pmarsh-scottlogic avatar Oct 04 '23 16:10 pmarsh-scottlogic

Adding my personal thoughts here:

  • I personally like enums here over objects. Having to number each value is annoying, as is reordering the values if we need to.
  • Enums as strings seems dodgy to me, I'd prefer all to be numeric.

gsproston-scottlogic avatar Nov 10 '23 13:11 gsproston-scottlogic

I see typescript enums being used in a few places in the code, e.g. DEFENCE_TYPES.

There is often a better way to achieve this using the power of types. Refer to https://www.typescriptlang.org/docs/handbook/enums.html#objects-vs-enums

Why are enums confusing? Because it is not always clear when you are referring to the enum name, and when to the (numeric or string) value. Numeric enums are particularly problematic as name and value are different, whereas String-valued enums seem a bit un-DRY when, as is often the case, the name is exactly the same as the value.

This is a placeholder issue for identifying all enums, and assessing which would be better represented using type narrowing and standard javascript objects. The first of these can then be used to better describe the issue, and a possible alternative. A likely candidate is #355.

I feel like those arguments stated for "why are enums confusing?" also apply to using objects. As far as I understand, they would act in exactly the same way! The article has its own argument in favour of objects which I don't understand but would trust blindly if told to :P

pmarsh-scottlogic avatar Nov 10 '23 13:11 pmarsh-scottlogic

I'll leave it to @chriswilty to defend the ticket :)

pmarsh-scottlogic avatar Nov 10 '23 13:11 pmarsh-scottlogic

@gsproston-scottlogic Consider this piece of code:

enum LEVEL_NAMES {
  LEVEL_1,
  LEVEL_2,
  LEVEL_3,
  SANDBOX,
}

// Using enum in a loop
Object.values(LEVEL_NAMES)
  .filter((value) => Number.isNaN(Number(value)))
  .map((value) => {
    // do something

What's that filter?! Why are we filtering out numeric values from an enum?

It's because of how typescript represents enums as objects when transpiled into javascript (as javascript does not have native enums):

var LEVEL_NAMES;
(function (LEVEL_NAMES) {
    LEVEL_NAMES[LEVEL_NAMES["LEVEL_1"] = 0] = "LEVEL_1";
    LEVEL_NAMES[LEVEL_NAMES["LEVEL_2"] = 1] = "LEVEL_2";
    LEVEL_NAMES[LEVEL_NAMES["LEVEL_3"] = 2] = "LEVEL_3";
    LEVEL_NAMES[LEVEL_NAMES["SANDBOX"] = 3] = "SANDBOX";
})(LEVEL_NAMES);

/*
LEVEL_NAMES now looks like this:
{
  "LEVEL_1": 0,
  "LEVEL_2": 1,
  "LEVEL_3": 2,
  "SANDBOX": 3,
  0: "LEVEL_1",
  1: "LEVEL_2",
  2: "LEVEL_3",
  3: "SANDBOX"
}
*/

Confusion reigns.

chriswilty avatar Nov 14 '23 14:11 chriswilty

@chriswilty Funny, I came across this exact bit of confusion yesterday. Took a wee while to figure out why my loop was behaving all weird haha

pmarsh-scottlogic avatar Nov 14 '23 14:11 pmarsh-scottlogic

@gsproston-scottlogic Consider this piece of code:

enum LEVEL_NAMES {
  LEVEL_1,
  LEVEL_2,
  LEVEL_3,
  SANDBOX,
}

// Using enum in a loop
Object.values(LEVEL_NAMES)
  .filter((value) => Number.isNaN(Number(value)))
  .map((value) => {
    // do something

What's that filter?! Why are we filtering out numeric values from an enum?

It's because of how typescript represents enums as objects when transpiled into javascript (as javascript does not have native enums):

var LEVEL_NAMES;
(function (LEVEL_NAMES) {
    LEVEL_NAMES[LEVEL_NAMES["LEVEL_1"] = 0] = "LEVEL_1";
    LEVEL_NAMES[LEVEL_NAMES["LEVEL_2"] = 1] = "LEVEL_2";
    LEVEL_NAMES[LEVEL_NAMES["LEVEL_3"] = 2] = "LEVEL_3";
    LEVEL_NAMES[LEVEL_NAMES["SANDBOX"] = 3] = "SANDBOX";
})(LEVEL_NAMES);

/*
LEVEL_NAMES now looks like this:
{
  "LEVEL_1": 0,
  "LEVEL_2": 1,
  "LEVEL_3": 2,
  "SANDBOX": 3,
  0: "LEVEL_1",
  1: "LEVEL_2",
  2: "LEVEL_3",
  3: "SANDBOX"
}
*/

Confusion reigns.

ok that is absolutely horrid

gsproston-scottlogic avatar Nov 14 '23 16:11 gsproston-scottlogic

Things that enums are good for:

  • Intellisense. You can type DEFENCE_TYPES. and then be able to see and autocomplete any the available defence types.
  • Searching. If you want to find places in the code that use our defence values, we can just search the codebase for the enum, or use handy ide tools to find instances of the token.
  • Renaming. If we find a better name for something, we can use the ide to dename all instances of a token. Without the enum, we'd have to "find and replace" which is more prone to error.

pmarsh-scottlogic avatar Nov 27 '23 17:11 pmarsh-scottlogic

Question: Need to decide what to do here @chriswilty

gsproston-scottlogic avatar Jan 04 '24 14:01 gsproston-scottlogic

  1. String-valued enums can very easily be converted to standard objects:

    const RagStates = {
      RED: 'Error',
      AMBER: 'Warning',
      GREEN: 'Ok',
    } as const;
    type Rag = keyof typeof RagStates; // 'RED' | 'AMBER' | 'GREEN'
    RagStates['AMBER']; // 'Warning'
    
  2. String-valued enums where the name and value are identical, can be converted to const arrays:

    const RagStates = ['RED', 'AMBER', 'GREEN'] as const;
    type Rag = typeof RagStates[number]; // 'RED' | 'AMBER' | 'GREEN'
    
  3. Numeric values of an enum, if genuinely needed, can be represented either by the index within an array ...

    const RagStates = ['RED', 'AMBER', 'GREEN'] as const;
    type Rag = typeof RagStates[number]; // 'RED' | 'AMBER' | 'GREEN'
    RagStates.indexOf('AMBER'); // 1
    

    ... or for non-zero-based enums, you can again use a standard object:

    const RagStates = {
      RED: 1,
      AMBER: 2,
      GREEN: 3,
    } as const;
    type Rag = keyof typeof RagStates; // 'RED' | 'AMBER' | 'GREEN'
    RagStates['AMBER']; // 2
    

    But remember that transferring the numeric value of an enum between UI and server, or storing that numeric value anywhere for later retrieval, is error-prone because inserting, removing or reordering enum values can very easily break things. It is far better to store the string value, as it has meaning.

chriswilty avatar Jan 25 '24 12:01 chriswilty