views:

344

answers:

11

Given the class definition below. How would one go about deciding whether the stub methods should be static or non-static?

class Point {
    private final int x;
    private final int y;

    public Point(int x, int y) {
        this.x = x;
        this.y = y;
    }

    // Should the methods add(), subtract() and inverseOf() be non-static ...

    public Point add(Point point) {

    }

    public Point subtract(Point point) {

    }

    public Point inverseOf() {

    }


    // Or static?

    public static Point add(Point point1, Point point2) {

    }

    public static Point subtract(Point point1, Point point2) {

    }

    public static Point inverseOf(Point point) {

    }
}
A: 

Use a static method when the body of the method does not depend on any one particular instance.

As one example, look at your add(Point, Point) method. You are adding together the two Points that are passed to the function as arguments, and returning another Point. Does this really need an internal this reference to some Point?

On the other hand, you have a method add(Point). Presumably this adds the function argument to the instance - in that case, you would have to make this an instance method so you have both Points.

Edit: I think I misunderstood, originally. Looking back, you have the correct signatures for both the static and non-static implementations. At this point, I would say it is a matter of style, as you know both will work correctly. How do you want your point class to be used? Think of whether it makes the code more intuitive to say Point a = Point.add(b, c) or Point a = b.add(c). Personally, I like the former, as it tells me that neither of the operands is going to be modified.

danben
To the downvoter: it is generally good practice to leave a comment explaining the downvote.
danben
Upvoted, specifically because I hate downvoters that don't explain why they think you are wrong... so you're left to assume it's only because they disagree with your opinion (rather than a factual issue).
RHSeeger
@RHSeeger - indeed. No way to know if it's a legitimate complaint or just a SCITE.
danben
A: 

It is naturally that these functions has to be non static. But if you doubt please refer to GRASP, they describe things like this.

According to GRASP Information Expert these functions shouldn't be static.

Despite the fact there is no direct information about the static method, there is

Information Expert will lead us to place a responsibility in classes with the most information required to fulfill it.

If you make you methods static you will move your logic further from actual data and will have to pass data to method.

Removing static will put logic closer to data it uses.

Mykola Golubyev
It doesn't appear to me that the GRASP page you link really gives any indication of whether the methods should be static or not, just that they should be methods of the class in question (which they are in both cases).
RHSeeger
Why do they have to be instance methods? Can you cite any particular reasons?
danben
Static method do not have access to instance data. According to Information expert you should place you methods to place where they have access. Removing static put logic closer to the data.
Mykola Golubyev
A: 

In your case it has to be non static unless you change signature to public static Point add(Point point1, Point point2).

EDIT : I got down voted. That's fine. I was not trying to give trivial suggestion like putting static in front method. In this case, an instance method is better, but there is really not singe answer. It just depends on your preference.

fastcodejava
I think you may not have read the question closely.
BobMcGee
A: 

I tend to run counter to the norm on this, but either way seems reasonable to me.

  • The methods should obviously be part of the Point method, since they deal specifically with points
  • For the methods that use two points, there's nothing about them that imply they need more information about one of the points than the other... So there's no push as to which instance the method would be a non-static member of.

For a language like Java, I'd go with static methods, specifically because of the second point above. For a language that has operator overloading (like Ruby), I'd go with an instance method to take advantage of that.

RHSeeger
To the downvoter: it is generally good practice to leave a comment explaining the downvote.
RHSeeger
+2  A: 

Semantically, the static approach seems to make a bit more sense. Both will of course work, but the non-static approach gives precedence to one point over another, and furthermore implies that point1 (the method which add is called on) may be modified as a result of the call.

As a developer using your classes, if I saw the following:

Point p1 = new Point(1,2);
Point p2 = new Point(2,3);

p1.Add(p2);

or..

Point p1 = new Point(1,2);
Point p2 = new Point(2,3);

Point.Add(p1, p2);

my natural inclination would be to assume that the add() method in the non-static version modifies point1 to add the result of point 2. With the static approach, it's more clear (although not guaranteed!) that the method is pure and the represenative points are not being modified.

Ryan Brunner
Going with non-static methods but changing names to plus and minus might be a good middle ground
Geoff Reedy
+9  A: 

I would go for instance methods. You then have the capacity of making the methods part of an inteface and override them. You would get the benefit when you have to deal with 2d points or 3d points and have some client code that doesn't really care and just need to perform operations on Points implementing the interface.

svachon
Was just typing the same answer myself. You're faster so +1 :-)
Mendelt
Only reason to have static methods would be if you are going to have lots of calls where a null Point instance is valid.
james
+3  A: 

I think it depends on what you are trying to accomplish. If you are providing a method that adds any two points together then you want a static method. But if you want a method that adds a point to a given Point instance then you want an non-static method.

If you do use a static methods then you could consider putting the static methods into a separate utility class (PointCalculator) that only contains static methods. This is similar to the Math class.

richs
Given that the class is immutable I don't understand what the difference is between "providing a method that adds any two Points" and "a method that adds a point to a given Point instance"
leftbrainlogic
A static method conveys that you are adding two points to create a new Point.A non-static method conveys that you are modifying the existing Point. Which isn't true of course because you are returning a new Point.For example a user could write p1.add(p2); (user might think they have added to p2 to p1 and the new value is in p1)instead of Point p3=p1.add(p2) butPoint p3=Point.add(p1, p2) is very clear. So this is a point for static.
richs
+3  A: 

I'd go for non-static methods which are more object oriented (yes, using too many static method breaks the benefit of objects like polymorphism, inheritance...), even if your Point is immutable. And actually, this would be consistent with the way classes like BigDecimal or BigInteger are designed. On top of that, static methods make classes harder to test so I prefer to avoid using them if possible, especially when it makes sense.

Pascal Thivent
A: 

These methods should be static because the Class itself lends itself to being created via the constructor and assigned values once due to x and y being final. This means you can create Points, but not manipulate their data going forward. The Add/Substract/Etc methods are utility methods which should not require an instance of Point to be used.

Nissan Fan
It is perfectly reasonable to define methods that look like mutators but return a new instance on an immutable object. This is a common functional programming practice and is a good way to write thread-safe Java as well.
Alex Miller
+1  A: 

If you're going to use Java and create objects, then stylistically, I think you should try to make maximal use of objects and data encapsulation. To me, that means leaving the data where it is (in the Point class) and not passing it to a separate method to deal with it. Make your objects work for you; not just have getters and setters. In fact, think hard about how you can avoid needing a getter at all.

It is perfectly common to have methods like add() and subtract() on an immutable class that return new instances of the immutable class. This is good style for FP-like programming and perfectly reasonable for a class like this. (See BigInteger or BigDecimal for good examples. DON'T see Date or Calendar for bad broken scary examples. :)

Keeping methods in the class lets you optionally define interfaces that these classes might implement, use the Decorator or Adapter pattern, write certain kinds of tests, etc.

Alex Miller
A: 

Making them static also makes it harder to unit test them! The only mocking framework I am aware of in .NET that could handle this is TypeMock.

If the intention is to make this class immutable, than you will be returning new Point objects in any accessor, call so making them static doesn't make much sense here.

Casey