views:

359

answers:

9

As a Swing developer for as many years as one can possibly be a Swing developer, I've identified a lot of patterns that I use in laying out components. For example, I often create components that are associated with a JLabel. I usually write:

JPanel panel = new JPanel(new BorderLayout());
panel.add(label, BorderLayout.NORTH);
panel.add(list, BorderLayout.CENTER);

I do this so often, that I've decided to create a class that contains my commonly-used layout idioms. Then I can simply say:

JPanel panel = LayoutPatterns.createNorthLabeledPanel(label, list);

...which reduces my typing load considerably.

So, now I have a class full of about 20 static methods. The class has no state - all context is passed in through the method parameters.

Besides Java's Math class, I haven't seen any classes that are composed entirely of static methods, and that have no state.

On one hand, this doesn't feel right. On the other hand, I don't see anything wrong with it.

Is this an okay pattern to use, or something that indicates a Code Smell? If this pattern were applied to a different domain, should I be concerned about multithreaded uses of a class of statics? Would you balk if you ever saw this in production-quality code?

+2  A: 

I dont see a problem with making a class like this. If the class makes sense in its normal use such that you would never need to instantiate an instance of it, then why not make the methods static?

I've written code similar to this myself.

Pete
+2  A: 

There is nothing wrong with a static only utility class, as long as things don't get out of hand. If the method you put in there don't require any state, there is no reason for your class to be instantiated.

As a matter of fact, the situiation is so common that in .Net, an extension method must be in a static class, as most of the time, a utility function is static and tries to extend another type functionality.

Coincoin
+1  A: 

I don't see any problem with your approach, in fact, I do these utility static classes all the time. Because this class doesn't have any state I think you'll have no problem in a multithreaded environment. If your concerned with the size this class is getting consider dividing it according to some types of swing components.

bruno conde
Good answer and, yes, the growing size of the class is a concern to me. I wonder how big is too big... I guess when it takes too long to quickly read through the API to find what you're looking for (if someone else were to use the class)
David
+4  A: 

I think this kind of thing usually has a bad code smell. However, I am with you, I don't see anything particularly wrong in this case.

I think you have explained and justified perfectly well your design.

Thinking about a possible alternative, maybe you could inherit JPanel and create NorthLabelJPanel (or maybe even better create a new class that contains a JPanel). However, I'm not sure if this would be worth the effort. I think your code would look more complicated this way, even when it may be the better way "by the book".

My 2 cents :)

Rafa G. Argente
I do agree with the bad code smell, though I could quite put my finger on it. But in response to your alternative, has-a relationships are favored over is-a relationships (where appropriate). Java has already developed their components using has-a relations, so I see no reason to undo that.
ph0enix
agree with both your suggestions, I might suggest also trying to keep these utility classes well organised and named so people reusing them can hopefully easily discover what they contain. And so that you can hopefully easily work with them yourself.
Robin
Interesting alternative but if all a subclass does is some work in the constructor, but not add/change and methods, it's probably an abuse of inheritance. Ideally the .add() method should be chainable but I see the static factory methods as the best solution here.
Outlaw Programmer
Good comments! I've edited my answer
Rafa G. Argente
+4  A: 

I think the only problem here is a language that forces you to invent a class where -gasp- global functions are totally appropriate. :-)

Ulrich Gerhardt
+1 - Made me smile.
David
A: 

VB.NET and now C# even have language support for static classes (VB calls them Modules) so you can have the compiler check that they are indeed all static.

reinierpost
A: 

Personally, I would prefer to see the code in the original form of what you've described. It would be much easier to tell at a glance exactly what the code is doing, and easier to maintain because if you ever wanted to change the layout of the panel, you wouldn't have to search for the proper method or create a whole new method to achieve what you want. Also, as you add more attributes to your components, the number of your static methods would grow at an exponential rate.

In the world of design patterns, I see the Builder pattern applied to cases like this, which it appears that is what the Java API is trying to achieve.

The basic rundown:

  1. Create a simple object, little or no decoration
  2. Add objects/attributes/etc. to it to make it look/do what you want

NOTE: The beauty is that you don't know what the end result will look like until you are done, which gives you a full range of flexibility.

Furthermore, I just read a response that discussed extending JPanel. From a design stand-point, remember to try to favor a has-a relationship instead of an is-a relationship whenever possible. Otherwise, you end up with a very flat class structure that looks identical to your static methods, and would suffer the same pitfalls as I described above.

Now with that said, I have written and used a large collection of what I call "Services", classes that have no state and only static methods (as you've described), and they have saved me from writing (and more importantly, duplicating) a lot of my basic routines. They can come in real handy, if used properly, but I tend to only use them for very basic (and common) routines, such as formatting text, copying arrays, converting float arrays to double arrays (in C++), or anything else that you would expect to find in a standard library.

It really just depends on how "static" your static methods really are. If you don't see them needing to be maintained or updated that often, then you could probably get away with locking them down to what you've described. However, if there's any chance of re-factoring in the future, I think you should prefer to leave it as flexibile as possible, which unfortunately means making the API calls manually each time.

Good luck!

ph0enix
+1  A: 

Seems like every project ends up with a static utility class sooner or later. For your particular example I would maybe look into a factory pattern. But I don't see anything wrong with what you are doing if it helps you get the job done and doesn't confuse others on your team.

NitroxDM
A: 

I think that if you are developing in an object-oriented environment, you have only two options:

  1. Make all the class methods static.
  2. Develop a regular class and use the singleton pattern.
Konamiman