-1

Which of the below is a good practice. Second looks more readable for me but would like to know how others think.

Solution 1 :

case colorElementEnum.ICON:
        color = colors.buttonText;
        if (isHighContrast) {
          if (!titleColor) {
            if (disabled) {
              color = colors.lightGray;
            }
          }
        }
        return color;

Solution 2 :

case colorElementEnum.ICON:
        color = colors.buttonText;
        if (isHighContrast && !titleColor && disabled) {
              color = colors.lightGray;
        }
        return color;
  
madu
  • 557
  • 6
  • 14
  • 1
    *" ... but would like to know how others think"* - What I think is that you are experienced enough to have (and trust) your own opinions on style and code readability :-). But as someone who has been using StackOverflow for some time, you should have realized by now that opinion-based questions are clearly off-topic. – Stephen C Aug 22 '21 at 09:03

1 Answers1

1

I prefer the latter option, because as it is said here, it is neat and readable, even if you have long names for variables, you can make it shorter and cleaner as so:

bool aa = FirstVeryLongConditionMethod(a) && SecondVeryLongConditionMethod(a);
bool bb = FirstVeryLongConditionMethod(b) && SecondVeryLongConditionMethod(b);
bool cc = FirstVeryLongConditionMethod(c) && SecondVeryLongConditionMethod(c);

if( aa && bb && cc)
{
   //This is again neat & readable
   //although you probably need to sanity check your method names ;)
}

Unless you have a certain functionality for your else condition, you can use above. As an another idea, you can use a specific function for checking your complicated checking (if you get). Thanks to @Torbjørn and @eoin-campbell, you can also find another ideas in that link.

AMK
  • 662
  • 6
  • 16