views:

588

answers:

9

When is it considered poor practice to use the static keyword in Java on method signatures? If a method performs a function based upon some arguments, and does not require access to fields that are not static, then wouldn't you always want these types of methods to be static?

+4  A: 

What you say is sort of true, but what happens when you want to override the behavior of that method in a derived class? If it's static, you can't do that.

As an example, consider the following DAO type class:

class CustomerDAO {
    public void CreateCustomer( Connection dbConn, Customer c ) {
       // Some implementation, created a prepared statement, inserts the customer record.
    }

    public Customer GetCustomerByID( Connection dbConn, int customerId ) {
       // Implementation
    }
}

Now, none of those methods require any "state". Everything they need is passed as parameters. So they COULD easily be static. Now the requirement comes along that you need to support a different database (lets say Oracle)

Since those methods are not static, you could just create a new DAO class:

class OracleCustomerDAO : CustomerDAO {
    public void CreateCustomer( Connection dbConn, Customer c ) {
        // Oracle specific implementation here.
    }

    public Customer GetCustomerByID( Connection dbConn, int customerId ) {
        // Oracle specific implementation here.
    }
}

This new class could now be used in place of the old one. If you are using dependancy injection, it might not even require a code change at all.

But if we had made those methods static, that would make things much more complicated as we can't simply override the static methods in a new class.

Eric Petroelje
+1, even stateless methods might belong to an instance, and be available for override.
erickson
+11  A: 

One reason why you may not want it to be static is to allow it to be overridden in a subclass. In other words, the behaviour may not depend on the data within the object, but on the exact type of the object. For example, you might have a general collection type, with an isReadOnly property which would return false in always-mutable collections, true in always-immutable collections, and depend on instance variables in others.

However, this is quite rare in my experience - and should usually be explicitly specified for clarity. Normally I'd make a method which doesn't depend on any object state static.

Jon Skeet
@Downvoter: Care to give a reason why?
Jon Skeet
Static is not about accessing the member fields or not. It is about class semantics. If a method applies to instances of the class, it must not be static. In your case, it is your collection instance that is read-only, not the class itself, so the function must be non-static. Static is really about class methods, for factories or utility functions.
Vincent Robert
Please, leave me the time to actually type my comment ;-)
Vincent Robert
If it takes a while to leave the comment, it may be worth doing that before casting the downvote :) (But thanks for commenting.)
Jon Skeet
Sorry for that, it was an impulsive down vote :)
Vincent Robert
I'm not actually sure which bit of your comment actually disagrees with what I've said - it's just expressed in a different way. I still think that it's *relatively* rare for a method to apply to an instance of the class, but not depend on its state. The only thing that can leave is the precise type of the object, which is where inheritance and overriding comes in.
Jon Skeet
I've been bit by exactly this problem before: I need to use a class in a testing or other scenario, but unable to override unwanted behaviour :-(
Roboprog
You dare downvote Jon Skeet? BURN THE HERETIC! j/k :D
MattC
@Roboprog: In that case I'd approach the testing differently. I *very rarely* override a method in a concrete class for the sake of testing. I'll implement interfaces or derive from abstract classes, but not concrete ones. Obviously it depends on the exact scenario, but I don't agree with uncontrolled inheritance/overriding :)
Jon Skeet
I use static methods whenever I can. Sometimes I need to move things around (like creating a utility class), and the static methods make that task easier to perform.
Ravi Wallau
@Jon: I'm (hopefully) not promoting spaghetti inheritance, just issuing a warning. If something you need to "transplant" makes a bunch of references to static methods, and you can't fix it, and you don't have some kind of "dependency injection" mechanism, it can suck. Having said that, the ironic part of this discussion is that I then turn right around after the earlier post, and write that very hour some static methods for debug display and logging in something I was working on. What a hypocrite I am! I'll go back and do an interface / singleton wrap later, if I have to :-)
Roboprog
A: 

That's right. Indeed, you have to contort what might otherwise be a reasonable design (to have some functions not associated with a class) into Java terms. That's why you see catch-all classes such as FredsSwingUtils and YetAnotherIOUtils.

Jonathan Feinberg
+8  A: 

In general, I prefer instance methods for the following reasons:

  1. static methods make testing hard because they can't be replaced,
  2. static methods are more procedural oriented.

In my opinion, static methods are OK for utility classes (like StringUtils) but I prefer to avoid using them as much as possible.

Pascal Thivent
+1 especially for the testing mention. We're using JUnit, and overriding methods in mock objects is a requirement.
hal10001
Regarding 2: OO is a tool, not a goal.
erikkallen
@erikkallen Agreed. But when I use a OO language, I like to use this tool.
Pascal Thivent
"static methods are more procedural oriented (and thus less object oriented)" not sure whether i understand that. A static method is still connected to its class and must be qualified when called from outside its class or statically imported, and if it doesn't need access to non-static members, why grant it?
Johannes Schaub - litb
What I mean is this style of code: Foo.bar(...); Bar.foo(...); ... i.e. procedural programming. I removed the *(and thus less object oriented)* which seems to be confusing.
Pascal Thivent
A: 

when you want to use a class member independently of any object of that class,it should be declared static.
If it is declared static it can be accessed without an existing instance of an object of the class. A static member is shared by all objects of that specific class.

Gnark
+11  A: 

Two of the greatest evils you will ever encounter in large-scale Java applications are

  • Static methods, except those that are pure functions*
  • Mutable static fields

These ruin the modularity, extensibility and testability of your code to a degree that I realize I cannot possibly hope to convince you of in this limited time and space.

*A "pure function" is any method which does not modify any state and whose result depends on nothing but the parameters provided to it. So, for example, any function that performs I/O (directly or indirectly) is not a pure function, but Math.sqrt(), of course, is.

More blahblah about pure functions (self-link) and why you want to stick to them.

I strongly encourage you to favor the "dependency injection" style of programming, possibly supported by a framework such as Spring or Guice (disclaimer: I am co-author of the latter). If you do this right, you will essentially never need mutable static state or non-pure static methods.

Kevin Bourrillion
So you'd argue against doing any I/O within a static method even if it does nothing with the state of the object, it's a sealed class, and you're passing in the object on which to perform the IO as an argument? I'd agree it'll still not be pure, but I don't see how making it an instance method helps in this case.
Jon Skeet
(I'd agree with mutable static fields generally being a bad idea, mind you.)
Jon Skeet
On a side note, to everyone else: expressing disagreement with Kevin is like expressing disagreement with Eric Lippert. I'm feeling nervous, almost expecting to be hit over the head with a giant cluestick. Please be gentle, Kevin...
Jon Skeet
Uh oh, I don't know this Eric Lippert, so now I'm paranoid that he's some kind of asshole. :)Here's the deal with your hypothetical static method that does I/O: it's fine.Oh, but the code that calls it? That code is not fine. That code is difficult to test. Since it uncontrollably performs I/O, my test needs to make sure that the state of things external to my program is set up in a particular way. Now I'm dependent on that setup, and if I forget it, I might be okay anyway depending on what order my tests are run in, and if I forget to restore pristine state later... etc.
Kevin Bourrillion
@Kevin: I agree that's a general problem with any non-pure function, but how does only performing I/O in instance methods help? For tests, I'd prefer to inject the streams/readers/writers/etc - but that's a different matter from the method being static or not. I suspect I'm missing a point somewhere.
Jon Skeet
@Jon if you did I/O from a static method, how would you redirect it to a different type of I/O? If you decide to always pass the type of I/O (say a socket, logfile or console) then why not include it in the object instance and make it non-static? I agree with @kevin that avoiding them in general is a good habit. In fact, I tend to think that pure functions are an indication of iffy design (they are useful for libraries where you don't know what business logic will be using them, but have little place in business logic code)
Bill K
@Bill: What's to say that it's naturally part of the state of the object? You may not even *need* an instance of the object in question - such as for utility methods, as you mention. In fact, I tend to try to actively *avoid* putting resource-laden types (which generally require cleanup) into an object's state. It means you need to pay much more attention to the lifetime of the object. I generally obtain the resources within a method, store them as local variables, quite possibly passing them to other (possibly static) methods, then clean them up. I find that's simpler.
Jon Skeet
@Jon Why not extend (or better yet contain) the object you were passing into the function and include the function in this new object? No difference in lifecycle--no difference at all except your code is organized better and you don't have to search for the "Log" method, it's right there in your logger--not some utility running around in some unrelated class... You'll also find that you probably needed this new object and there are dozens of other things you thought were unattached functions strewn across your code that belong there. (at least that's my experience).
Bill K
@Bill K: For one thing, because I'm not *specializing* that type, I'm just *using* it. For another, it just doesn't work if I want to be able to be passed *any* input stream (for example) and load data from it. I don't want to force people to use InputStreamWhichKnowsHowToLoadContacts for example. My experience is that using inheritance when you're not really specializing behaviour is a bad idea - I far prefer composition.
Jon Skeet
@Jon That's why I suggested composition (as in "better yet contain")--I totally agree. But I can't figure out why you are favoring passing objects to functions over methods on new objects, but I guess it's just an opinion. I personally find most functions to be a bad OO code smell unless you are working on low level generic tooling to be used for multiple applications...
Bill K
@Bill K: We may be talking at cross-purposes. I suspect we'd have to have a concrete example to actually find out.
Jon Skeet
A: 

An additional annoyance about static methods: there is no easy way to pass a reference to such a function around without creating a wrapper class around it. E.g. - something like:

FunctorInterface f = new FunctorInterface() { public int calc( int x) { return MyClass.calc( x); } };

I hate this kind of java make-work. Maybe a later version of java will get delegates or a similar function pointer / procedural type mechanism?

A minor gripe, but one more thing to not like about gratuitous static functions, er, methods.

Roboprog
+1  A: 

Static methods are usually written for two purposes. The first purpose is to have some sort of global utility method, similar to the sort of functionality found in java.util.Collections. These static methods are generally harmless. The second purpose is to control object instantiation and limit access to resources (such as database connections) via various design patterns such as singletons and factories. These can, if poorly implemented, result in problems.

For me, there are two downsides to using static methods:

  1. They make code less modular and harder to test / extend. Most answers already addressed this so I won't go into it any more.
  2. Static methods tend to result in some form of global state, which is frequently the cause of insidious bugs. This can occur in poorly written code that is written for the second purpose described above. Let me elaborate.

For example, consider a project that requires logging certain events to a database, and relies on the database connection for other state as well. Assume that normally, the database connection is initialized first, and then the logging framework is configured to write certain log events to the database. Now assume that the developers decide to move from a hand-written database framework to an existing database framework, such as hibernate.

However, this framework is likely to have its own logging configuration - and if it happens to be using the same logging framework as yours, then there is a good chance there will be various conflicts between the configurations. Suddenly, switching to a different database framework results in errors and failures in different parts of the system that are seemingly unrelated. The reason such failures can happen is because the logging configuration maintains global state accessed via static methods and variables, and various configuration properties can be overridden by different parts of the system.

To get away from these problems, developers should avoid storing any state via static methods and variables. Instead, they should build clean APIs that let the users manage and isolate state as needed. BerkeleyDB is a good example here, encapsulating state via an Environment object instead of via static calls.

toluju
That's a very coherent argument against static state, but not against static methods in general. Most static methods I write only use their parameters. Admittedly I do tend to use static logger variables, which I agree has issues...
Jon Skeet
Right, that's what I was trying to outline in the first paragraph - static methods that don't alter any state outside of their parameters are usually fine, but static methods that manage state can be troublesome. So I guess as a quick rule of thumb for deciding when to write a static method, you should see if any state will be affected by the method.
toluju
A: 

Two questions here 1) A static method that creates objects stays loaded in memory when it is accessed the first time? Isnt this (remaining loaded in memory) a drawback? 2) One of the advantages of using Java is its garbage collection feature - arent we ignoring this when we use static methods?

Subramanian