Is duplicating files to avoid programming branching a good or anti pattern?

Question

for example, suppose I need to play different sounds according to "grade":

file list:

fairSound.mp3
goodSound.mp3
excellentSound.mp3

code:

showResult(grade){
  if(grade==0 || grade==1){
    SoundUtility.play(fairSound);
  }else if(grade==2 || grade==3){
    SoundUtility.play(goodSound);
  }else if(grade==4){
    SoundUtility.play(excellentSound);
  }
}

but I don't want any branching at this case, so my question is, is it a good or an anti pattern to duplicate sounds:

file list:

sound0.mp3 (copy from fairSound.mp3)
sound1.mp3 (copy from fairSound.mp3)
sound2.mp3 (copy from goodSound.mp3)
sound4.mp3 (copy from goodSound.mp3)
sound4.mp3 (excellentSound.mp3)

so that it doesn't need to write any branching case in the program in this case:

showResult(grade){
  SoundUtility.play(sound[grade]);
}

? Is it violating DRY principle?

Note: I also don't want to something like:

var arr=[fairSound.mp3,fairSound.mp3,goodSound.mp3,goodSound.mp3,excellentSound.mp3];

into my code because when I want to change grade 0 to play fairSound.mp3, I still need to modify the code

Answer 1

If changing the sound files for each single grade without modifying the code is the requirement here, I would externalize the configuration (mapping).

Create entries in your configuration mechanism (config files or database) which contain the sound file names for each grade.

Instead of duplicating the sound files I would rather list the same file name multiple times.

Example for an .ini file:

[resultSounds]
0=fairSound.mp3
1=fairSound.mp3
2=goodSound.mp3
3=goodSound.mp3
4=excellentSound.mp3

Read your configuration file into some data structure (like an array or a hash map) and use that in your method to find the file to play.

Answer 2

It is neither good or bad, and definitely neither a "pattern" nor an "anti-pattern". This is all about what your (non-functional) requirements are.

You wrote you want to be able "to change grade 0 to play fairSound.mp3" (or another sound), without changing the code. This looks to me as if it is more or less "by chance" that some grades have the same sound. So you could treat them as if those soundfiles were all different, in which case it is clear you should duplicate the mp3 files. This makes sure whenever you change the sound of one grade, the other grades are not affected.

However, if you want to be able to exchange the mp3 file of one sound later, and make sure all usages of this particular file will be updated later as well (for all the grades where it was used), you should avoid duplication. You can achieve this most easily by the kind of array you mentioned in your question. If you want to avoid having to change the code afterwards, put the array into an external configuration file outside of the code.

TLDR; think about the kind of "changeability" you want to achieve: either optimize for changeability "per grade", or for changeability per "sound category".

Answer 3

Is using the { character twice in your program a violation of DRY? I hope you will say, naturally not.

When evaluating if something is a violation of DRY, you must not only consider the similarity between the pieces of code (or the files in your case), but also the reason why it looks so similar.

If it looks similar because it is required to be the same (the same functionality is coded twice, or grades 0 and 1 must always play the same sound), then you have spotted a violation of DRY.
If it looks similar today, but it is completely fine and expected that a new requirement can come in that will cause it to be different, then the similarity is incidental and not a violation of DRY. For example, if it is reasonable that there might be a request to play different sounds for grade levels 0 and 1, then the fact that the files sound0.mp3 and sound1.mp3 are currently the same is not an issue.

Answer 4

What you're doing is mapping the sound file with an identifier in your program (literally an index in your array). This isn't an antipattern if each individual file is unrelated with another.

However now you're introducing grade quality into the equation, so you end up with groups of sounds related to each other. In order to fix this, you're having to map your mapping. This hardly makes sense, as the mapping is arbitrary and entirely up to you on how to structure, so it makes sense to rearrange in such a way as to take this into consideration.

Logical sound grouping

One possible way might be creating subgroups of sounds. Rather than have an array of sounds [A, B, C, D], with A, B, C being different qualities of the same sound, you could group it as [[A, B, C], [D]]. The grade being passed now directly corresponds with the index in your subarray. Notice that D stands alone. If a high grade of sound D is requested, you can grab item at index min(gradeArr.length - 1, gradeParam), or in other words, grab the appropriate grade quality or the maximum available otherwise.

File naming approach

A second approach might be give a logical grouping to files belonging to the same sound grade. So for sound "birdChirping", to represent multiple sound grade qualities, the files birdChirping_01.mp3, birdChirping_02.mp3, and birdChirping_03.mp3 might be present in the same folder. In your program, you have only "birdChirping" as its name. Give the name and the sound grade, you can retrieve and play the appropriate file. In your program, rather than map every single sound, you map only the sound type (or in this case "birdChirping").

This is more dynamic in that you don't have to switch up your program for every new file, however you might run into problems when the caller asks for a sound grade which doesn't exist. You may need to list all files named birdChirping_*.mp3 to know what sound qualities are available done in this way. So while more dynamic, it is also less robust, so you should take that into consideration.

Conclusion

Make the data work for you. Don't be afraid to rearrange in more helpful configurations if you need to. You're right to think that the program shouldn't have ugly if else sections for grade. This is how you fix it. You make the program work off the data it's given without any further checks or mappings.

Hope that helps you!

Answer 5

The other answers are excellent and I particularly agree with @Neil regarding the loss of robustness if your code now relies on files that might not be there. I just wanted to add that assuming that you have the configuration testing setup to deal with these issues (e.g. crash verbosely on start-up if any required files are missing), I would argue that managing the files by filenames is not a bad idea at all.

This is a particularly good approach in a case when different files are needed in different environments. So each environment would have a different configuration of these files, and to save space and simplify the configuration, you don't actually need to copy them, but rather can instead rely on symlinks. For example, you could have the following file structure, inspired by that used in apache for site configurations:

├── sounds-available
│   ├── excellentSound.mp3
│   ├── fairSound.mp3
│   └── goodSound.mp3
└── sounds-enabled
    ├── excellentSound.mp3 -> ../sounds-available/excellentSound.mp3
    ├── fairSound.mp3 -> ../sounds-available/fairSound.mp3
    └── goodSound.mp3 -> ../sounds-available/fairSound.mp3

To sum up, this pattern moves the configuration flexibility out of the program into the environment itself, which can then be automated and source-controlled by a configuration management system such as Chef/Puppet/Ansible. There are pros and cons to this approach, but I would argue it's a very valid choice for some use-cases.

Answer 6

You want to replace some perfectly fine code (well, it should be a switch handling the default value) with something much more complicated in order to follow some principle. In the current code, everything is in one place, so any changes will be localised.

And yes, if your specs change, you change the code.

Is it a good pattern or an anti-pattern? It’s not a pattern at all. Nobody else thinks it is a good idea.