Should the ability to locate where code is being used be a consideration?

Question

I was modifying a class whose job is to load other classes from the same directory. Basically, everytime a new class is added you need to add a new CommandClass; statement to this class for it to be loaded.

I thought that it's strange and violates "Closed for modification" principle, so instead of adding another line to existing 70 of them, I changed it to autoload all these classes.

After a while two people argued that with autoloading those classes, they can't find where these classes are used, i.e. they can't grep for the class names to locate files that are using them.

Considering that others have already been doing these for more than seventy times, I reverted my change (and add another one of those lines). However, I'm still not sure, does that argument holds any real weight? What better way is there to satisfy their need to locate code?

Answer 1

In general, whenever using dynamic loading or accessing techniques, you will have to deal with the problem that your typical search, refactoring or cross-referencing tools cannot easily determine any more which code is used where, what to refactor or from where a piece of code is called.

So whenever you consider to use such a technique, be aware that this is a trade-off - on the pro side, your code gets more DRY, or it will conform to the OCP, and so becomes more easily extendable. The latter is especially helpful when you create a library or framework where the user of that lib is not able to change or extend the lib source code arbitrarily. On the con side, the whole mechanism becomes less obvious. If you use these techniques too often, there is a certain risk the code might become hard to understand or convoluted. Therefore, you will have to make a decision on a per-case base, which approach will serve you better in total.

In your specific case, it sounds there might be a little bit of superstition of your colleagues involved - do they really require to find the part of the code which holds the "load" class by using grep and search for a specific class name? If, for example, one name of the classes will change, your autoloader won't have to be changed, so no need to grep for it. And 70 repetitions of similar code is surely a code smell - when in the future you get a requirement which causes the same kind of change to every of those 70 lines, then an autoloader will show its real value.

On the other hand, changing the loader everytime a new class is added does not seem to be much of a problem in your current situation, because you have full control of the source code, and there is hopefully not really much code repeated. I guess when you have to add a class, it cannot easily be forgotten to extend the loader (your system would probably not be able to run the code of the new class). And when one of your team wants to keep some old versions of the classes in the directory, but does not want those to be loaded (for example, for testing purposes), a "whitelist" of classes to be loaded can have some benefits, too.

In the end, you have to consider these two sides and make a decision for your case.