30

Suppose I have a class like this: 

public class Car {

    private double distanceDriven;

    public void drive(double miles){
        distanceDriven += miles;
    }

    public void driveInCanada(double kilometer){
        distanceDriven += convertToMiles(kilometer);
    }

    private double convertToMiles(double km){
        return km*0.621371192;
    }   
}

You can see that convertToMiles is:

  • not using any instance variables
  • is only used inside the class

Should it be declared as static? This does not change the functionality of the the function at all (see above). I think that it may affect:

  • readability
  • performance
  • other?

Should the convertToMiles function look like: 

    private double convertToMiles(double km){

or

    private static double convertToMiles(double km){
Paul Bellora
  • 54,340
  • 18
  • 130
  • 181
sixtyfootersdude
  • 25,859
  • 43
  • 145
  • 213
  • In C++ this would change the syntax of the call: instead of `mycar.convertToMiles(x)` you'd need `Car::convertToMiles(x)`. I'm curious if that's the case in Java? – Mark Ransom Aug 29 '11 at 19:29
  • 2
    @Mark: He's talking about private methods, and in this case from inside the object, there's no change to the syntax of the call. – Ben Zotto Aug 29 '11 at 19:31
  • @quixoto, good observation - the same would be true in C++. – Mark Ransom Aug 29 '11 at 19:34
  • While the author was probably hoping for a definitive answer, I think this is really much more of a discussion than a question. Like all good questions meant to prompt discussion, the answer is, 'it depends'. Is performance an issue? Is strict OOP an issue? (I know some people think of breaking OOP principles as a sin, but lets face it, sometimes certain principles are just not practical to follow.) – Trevor Aug 29 '11 at 20:02

6 Answers6

18

For maximum stylistic hygiene, yes, private methods that don't use any object state, but only make sense inside the object, should be static.

That's the clearest (and strictest) way of indicating how they operate, and it will helpfully force you to be careful about your design around method-boundaries, and to think twice if you decide to go change one of them later to use object data.

FWIW, I don't suspect there's a relevant performance impact here (in theory the static is easier to call due to no implicit this reference). Also, you could go nuts being strict about this in your codebase, but it's certainly a reasonable goal to have.

N.B. Public methods require more consideration before marking them static; those can't change down the road without impact to callers, so "defaulting to tightness" isn't always the right choice.

Ben Zotto
  • 70,108
  • 23
  • 141
  • 204
6

If you're asking yourself this, they your design is already shaky. You should rip all those "static" functions out of the class and put them in a generic, reusable algorithm container static class.

Look at your code, what does convertToMiles have to do with a car? That's a generic algorithm that can be reused in multiple functions.

Blindy
  • 65,249
  • 10
  • 91
  • 131
  • 1
    Is your underlying thinking that *all* private static methods are problematic? If so, I'm interested in hearing more detail about how to think about less trivial examples than this one. (If not, this might have better served as a comment.) Thanks. – Ben Zotto Aug 30 '11 at 03:16
  • 1
    Not all, just most. I've had a few classes where I've ended up writing static menthods that are actually part of the class' logic without using any of it's state, but that's so rare, and obviously not the OP's case. – Blindy Aug 30 '11 at 12:14
4

Using static might make a performance difference, however this is less likely if it is inlined as it won't be called as much.

static is useful as it makes it clear you are not accessing any member fields. This has picked up some bugs for me in the past when I marked a method as static but this produced an error (because it shouldn't have been using a member field)

You can get creative with the design and add layers and complexity which might be useful one day, but I would go with the YANGI principle and say it is unlikely you are going to want to change how kilo-meters are converted to miles, or if you do change it you are unlikely to want more than one way of doing it.

Peter Lawrey
  • 525,659
  • 79
  • 751
  • 1,130
4

A definitive NO for ALL such methods.

For example it is perfect legal that such a method calculates an result (return value) only on its arguments, and the author would like to allow others to change the calculation in a subclass. (This is some kind of Template Method pattern.) -- And overriding a class can be only done if they are ..not.. static.

Btw: if you change your question and ask only for private methods, then I could not argue this way. But you asked for all kind of methods.

Ralph
  • 118,862
  • 56
  • 287
  • 383
  • 1
    Excellent point inheritance is one thing that I had not considered. I wish I could upvote this twice. However for a private method this should not matter. – sixtyfootersdude Aug 29 '11 at 20:39
2

yes. Use static methods when you can.

Michał Šrajer
  • 30,364
  • 7
  • 62
  • 85
  • 1
    -1 Using static method prevents sub-classes from overwriting them, which limits flexibility and extensibility. – Philipp Wendler Aug 30 '11 at 07:34
  • 2
    @Philipp If you need to override a method then you cannot cannot set it as static. I said "use static when you *can*". Too much flexibility sometimes is bad. This is why String class is final. It's limiting flexibility by design. – Michał Šrajer Aug 30 '11 at 11:50
1
private static double convertToMiles(double km){}

This will be the right one for your program code as convertToMiles() method has nothing to do with the instance variable.

But keep in mind this method is not reusable in other class by non-static members, if yes then the very purpose of static wont serve, as static avoids multiple object creation and memory wastage.

Hugo Dozois
  • 8,147
  • 12
  • 54
  • 58
Asim Parveez
  • 11
  • 1