DEV Community

Discussion on: 5 easy wins to refactor even the ugliest code

Collapse
 
nssimeonov profile image
Templar++

I don't understand what's wrong with the switch statement? Instead of assigning the variable you could have return for each case and this will also save you the break statement as well.

getBackgroundArt(track: Track): BackgroundImage {
 let backgroundImage: BackgroundImage;
 switch(track.getGenre()) {
   case "hiphop":
     return {dimension: 'small', 'url': 'https://unsplash.com/photos/Qcl98B8Bk3I'};
   case "jazz":
     return {dimension: 'small', 'url': 'https://unsplash.com/photos/dBWvUqBoOU8'};
   case "rap":
     return {dimension: 'small', 'url': 'https://unsplash.com/photos/auq_QbyIA34'};
   case "country":
     return {dimension: 'small', 'url': 'https://unsplash.com/photos/RnFgs90NEHY'};
   default:
     return {dimension: 'small', url : 'https://unsplash.com/photos/PDX_a_82obo'};
 }
}

Moreover instead of calling track.getGenre() once, you do that multiple times, and functions may implement complex logic.

Collapse
 
wstone profile image
Will Stone

Was going to comment with the same. You can even ditch line 2 let backgroundImage: BackgroundImage; now this variable isn't used.

Collapse
 
nssimeonov profile image
Templar++ • Edited

Yep, forgot that one.. lint usually reminds me in this case :)

Collapse
 
mlevkov profile image
Mikhail Levkovsky

100% agreed about calling track.getGenre() once, should definetely do that.
as for that last example, that's actually the good one, where you return from each case, which IMO helps keep the flow of the code linear and simple