Creating a Catch-All AppToolbox Class - Is this a Bad Practice?

Question

Never sure where to place functions like:

String PrettyPhone( String phoneNumber ) // return formatted (999) 999-9999
String EscapeInput( String inputString ) // gets rid of SQL-escapes like '

I create a Toolbox class for each application that serves as a repository for functions that don't neatly fit into another class. I've read that such classes are bad programming practice, specifically bad Object Oriented Design. However, said references seem more the opinion of individual designers and developers more than an over-arching consensus. So my question is, Is a catch-all Toolbox a poor design pattern? If so, why, and what alternative is there?

Answer 1

There is nothing wrong with this. One thing is try to break it up into logical parts. By doing this you can keep your intellisense clean.

MyCore.Extensions.Formatting.People
MyCore.Extensions.Formatting.Xml
MyCore.Extensions.Formatting.Html

Answer 2

I think the reason it's frowned upon is because the "toolbox" can grow and you will be loading a ton of resources every time you want to call a single function.

It's also more elegant to have the methods that apply to the objects in the actual class - just makes more sense.

That being said, I personally don't think it's a problem, but would avoid it simply for the reasons above.

Answer 3

Great question. I always find that any sufficiently complex project require "utility" classes. I think this is simply because the nature of object-oriented programming forces us to place things in a neatly structured hierarchical taxonomy, when this isn't always feasible or appropriate (e.g. try creating an object model for mammals, and then squeeze the platypus in). This is the problem which motivates work into aspect oriented programming (c.f. cross cutting concern). Often what goes into a utility class are things that are cross-cutting concerns.

One alternative to using toolbox or utility classes, are to use extension methods to provide additional needed functionality to primitive types. However, the jury is still out on whether or not that constitutes good software design.

My final word on the subject is: go with it if you need, just make sure that you aren't short-cutting better designs. Of course, you can always refactor later on if you need to.

Answer 4

In these examples I would be more inclined to extend String:

class PhoneNumber extends String
{
     public override string ToString()
     {
         // return (999) 999-9999
     }
}

If you write down all the places you need these functions you can figure out what actually uses it and then add it to the appropriate class. That can sometimes be difficult but still something you should aim for.

EDIT:

As pointed out below, you cannot override String in C#. The point I was trying to make is that this operation is made on a phone number so that is where the function belongs:

interface PhoneNumber
{
    string Formatted();
}

If you have different formats you can interchange implementations of PhoneNumber without littering your code with if statements, e.g.,

Instead of:

if(country == Countries.UK) output = Toolbox.PhoneNumberUK(phoneNumber);
else ph = Toolbox.PhoneNumberUS(phoneNumber);

You can just use:

output = phoneNumber.Formatted();

Answer 5

I think a static helper class is the first thing that comes to mind. It is so common that some even refer to it as part of the object-oriented design. However, the biggest problem with helper classes is that they tend to become a large dump. I think i saw this happen on a few of the larger projects i was involved in. You're working on a class and don't know where to stick this and that function so you put it in your helper class. At which point your helpers don't communicate well what they do. The name 'helper' or 'util' itself in the class name doesn't mean anything. I think nearly all OO gurus advocate against helpers since you can very easily replace them with more descriptive classes if you give it enough thought. I tend to agree with this approach as I believe that helpers violate the single responsibility principle. Honestly, take this with a grain of salt. I'm a little opinionated on OOP :)

Answer 6

My experience has been that utility functions seldom occur in isolation. If you need a method for formatting telephone numbers, then you will also need one for validating phone numbers, and parsing phone numbers. Following the YAGNI principle, you certainly wouldn't want to write such things until they're actually needed, but I think it's helpful to just go ahead and separate such functionality into individual classes. The growth of those classes from single methods into minor subsystems will then happen naturally over time. I have found this to be the easiest way to keep the code organized, understandable, and maintainable over the long term.

Answer 7

When I create an application, I typically create a static class that contains static methods and properties that I can't figure out where to put anywhere else.

It's not an especially good design, but that's sort of the point: it gives me a place to localize a whole class of design decisions that I haven't thought out yet. Generally as the application grows and is refined through refactoring, it becomes clearer where these methods and properties actually ought to reside. Mercifully, the state of refactoring tools is such that those changes are usually not exceptionally painful to make.

I've tried doing it the other way, but the other way is basically implementing an object model before I know enough about my application to design the object model properly. If I do that, I spend a fair amount of time and energy coming up with a mediocre solution that I have to revisit and rebuild from the ground up at some point in the future. Well, okay, if I know I'm going to be refactoring this code, how about I skip the step of designing and building the unnecessarily complicated classes that don't really work?

For instance, I've built an application that is being used by multiple customers. I figured out pretty early on that I needed to have a way of separating out methods that need to work differently for different customers. I built a static utility method that I could call at any point in the program where I needed to call a customized method, and stuck it in my static class.

This worked fine for months. But there came a point at which it was just beginning to look ugly. And so I decided to refactor it out into its own class. And as I went through my code looking at all the places where this method was being called, it became extremely clear that all of the customized methods really needed to be members of an abstract class, the customers' assemblies needed to contain a single derived class that implements all of the abstract methods, and then the program just needed to get the name of the assembly and the namespace out of its configuration and create an instance of the custom features class at startup. It was really simple for me to find all of the methods that had to be customized, since all I needed to do was find every place that my load-a-custom-feature method was being called. It took me the better part of an afternoon to go through the entire codebase and rationalize this design, and the end result is really flexible and robust and solves the right problem.

The thing is, when I first implemented that method (actually it was three or four interrelated methods), I recognized that it wasn't the right answer. But I didn't know enough to decide what the right answer was. So I went with the simplest wrong answer until the right answer became clear.

Answer 8

I posted a comment, but thought I'd elaborate a bit more.

What I do is create a Common library with namespaces: [Organisation].[Product].Common as the root and a sub namespace Helpers.

A few people on here mention things like creating a class and shoving some stuff they don't know where else to put in there. Wrong. I'd say, even if you need one helper method, it is related to something, so create a properly named (IoHelper, StringHelper, etc.) static helper class and put it in the Helpers namespace. That way, you get some structure and you get some sort of separation of concerns.

In the root namespace, you can use instance utility classes that do require state (they exist!). And needless to say also use an appropriate class name, but don't suffix with Helper.