views:

393

answers:

10

I'm writing an API for creating geometric shapes, and I'm running into some difficulties naming my methods.

Let's take a simple case: Creating a circle. Most of us might be familiar with a method like graphics.drawEllipse(x, y, w, h). To draw a circle, you need to know the top left coordinate, and the width and height of the circle.

My API is intended to make it easy for a developer to draw shapes using a variety of information, without doing a lot of math - which is trivial for circles, but more complicated for other shapes. For example, you should also be able to draw a circle given its center coordinates and radius, or the top left and bottom right coordinates.

So I have a Circle class with factory methods like:

Circle.createWithCenterAndRadius(cx, cy, r)
Circle.createWithBoundingBox(x1, y1, x2, y2)
Circle.createWithWidthAndHeight(x, y, w, h)

I feel like there might be a "code smell" here, but I'm not sure. On the one hand, these factory methods are necessarily descriptive. On the other hand, I can forsee these method names getting out of control. For example, how would I name a Triangle factory method that creates a triangle given a point, the length of one side, an angle, and the length of another side? Triangle.createWithPointSideAngleAndSide(x, y, side1, angle, side2)? Is that just evil?

If you were to use this API, would method names like this be okay to you? Do you have advice on how I can make the method names more sane?

A: 

Yes, this is more of a meta-answer, but I suggest you take a peek at how naming is done in Apple's Cocoa.

Jan Jungnickel
+11  A: 

It is ok in any language that doesn't support named parameters. If the language supports named parameters, I like more the short Create and just have obvious parameters names.

For a language with named parameters, you would:

Circle.Create(
   centerX = cx, 
   centerY = cy, 
   radius = r
);

Another more involved option, would be a fluent interface like (but that is probably too much):

circleBuilder.Center(cx,cy).Radius(r)
circleBuilder.Center(x,y).Width(w).Height(y)
circleBuilder.BoundWith().Left(x1,y1).Right(x2,y2)

Center returns an instance of an intermediate class that only allows Radius or Width. And BoundWith returns one that only allows Left.

eglasius
That's an awful lot of code to create. I think you would waste a lot of time creating that code and then attempting to genericize it so it also works when creating a rectangle or a triangle.
jmucchiello
@Joe, updated it from "but that might be too much" to "but that is probably too much".
eglasius
+13  A: 

You might change your circle methods to

Circle.FromCenterAndRadius(...)
Circle.FromBoundingBox(...)
Circle.FromWidthAndHeight(...)

It implies that you're creating circles from their different representations in a kind of concise way...

Jason Punyon
+1 I like the improvement :)
eglasius
+1  A: 

For languages that don't support named parameters, would it be cleaner to make the method name something very simple like Circle.create and then just add an additional input flag string (like "center" or "bounding") that indicated how the input values should be interpreted for cases that are hard to discriminate based only on input variable number and type? Drawbacks to this would be that it requires extra logic inside of the method to handle different types of input arguments and also requires that the user remember the flag options.

gnovice
+1  A: 

I would have methods CreateTriangle and have the overloads show the different pieces of information required.

E.g.

Circle.CreateCircle(cx, cy, r)
Circle.CreateCircle(point1, point2)
Circle.CreateCircle(point, width, height)
ck
+1: Agreed. This is how I would do it as well... Let the overloaded parameters speak for themselves.
Cerebrus
er, you have two methods here with the same parameters (assuming all the parameters are some integer type).
slim
you ars correct, it would be better to use a Point class rhather than two ints to differentiate
ck
You could actually use both Point and Size to make it even clearer. (Point,radius),(Point,Point) and (Point, Size)
Lars Mæhlum
+8  A: 

I think there is nothing wrong with your descriptive methods - they are the compact and describe exactly what's going on. The users of the library will have no doubt about the function of your methods, neither the maintanance programmers.

You could also apply some design pattern here if you are really worried about exposing a large number of factory methods - like having factory methods with property classes. You could have a CircleProperties class with properties like CenterX, CenterY, Radius, (bool)UseCenterX, (bool)UseCenterY etc and then you pass this to the public factory method which will figure out which (private) factory method to use.

Assuming C#:

var circleProperties = new CircleProperties()
{
   CenterX = 10,
   CenterY = -5,
   Radius = 8,
   UseCenterX = true,
   UseCenterY = true,
   UseCenterRadius = true
};

var circle = Circle.Create(circleProperties);
DrJokepu
Do you really need the "UseX" properties? Couldn't you simply check which properties have been set (are non-null)? Or set them in setters for the other properties (e.g., setCenterX() sets UseCenterX)?
TMN
TMN: You can, but then you really have to pick your magic numbers correcly... Obviously, 0.0 is a totally valid coordinate. Setting these booleans automatically in the appropriate setter is a good idea though.
DrJokepu
+6  A: 

My first instinct is to have more types, which would allow for more intuitive method overloading.

// instead of Circle.createWithCenterAndRadius(cx, cy, r)
Circle.create( new Point(cx,xy), r);

// instead of Circle.createWithBoundingBox(x1, y1, x2, y2)
Circle.create( new Point(x1,y1), new Point(x1,y1) );
// or even...
Circle.create( new Box(p1,p2));

// instead of Circle.createWithWidthAndHeight(x, y, w, h)
Circle.create( new Point(x,y), w, h);

As well as Point, you could define Distance (which would allow for different units)

If this style suits you, consider why you need a factory method instead of a constructor.

Circle c = new Circle(new Point(cx,xy), r);
slim
A: 

Would it be useful to abbreviate some of the words to shorten it?

e.g From..

Triangle.createWithPointSideAngleAndSide(x, y, side1, angle, side2)

To...

Triangle.createWithPointSAAS(x, y, side1, angle, side2)

or...

Triangle.createWithPntSidAngAndSid(x, y, side1, angle, side2)

Of course having descriptive names is good an i dont see much of a problem with how your going about it.

kevchadders
I would remove the and's from all of those method names. They don't do anything except improve how it sounds spoken aloud. Who reads code aloud?
jmucchiello
A: 

I know, I know. This sounds completely crazy for you C/C++/Java people, but the examples given in the question and in all those answers clearly demonstrate what a bad, bad convention CamelCaseNaming really is.

Let's take another look at the original example:

Circle.createWithCenterAndRadius(cx, cy, r)  
Circle.createWithBoundingBox(x1, y1, x2, y2)
Circle.createWithWidthAndHeight(x, y, w, h)

And now let's get rid of that camel case notation

Circle.create_with_center_and_radius(cx, cy, r)  
Circle.create_with_bounding_box(x1, y1, x2, y2)
Circle.create_with_width_and_height(x, y, w, h)

This may seem terribly unfamilar, but be honest: which version is easier to read?

innaM
Not a bad example--it actually is slightly easier to read--but completely irrelevant. It's not like anyone's going to switch or anything (and MAN would it be bad if someone did!!) So what are you trying to accomplish?
Bill K
Note: When I'm in Java I use Java syntax, when I'm in Ruby I used Ruby syntax. I felt no need to switch any users of one to the other--both are quite readable. ps. Although the ruby is a little easier to read, the java is a good deal easier to type.
Bill K
A: 

Your instinct is correct--the entire pattern of creating things this way is--iffy.

Unless these are used just once or twice, they are going to become pretty messy. If you were creating a shape with 5 circles and 3 triangles, it would be a mess.

Anything beyond a trivial example would probably be best done with some kind of data-driven implementation.

Towards those ends, having it take a string, hash or XML to define your shapes might be extremely useful.

But it all depends on how you expect them to be used.

I have the same kind of issues with creating Swing controls in Java. You end up with line after line of "new Button()" followed by a bunch of .set property calls as well as a line of code to copy the value to an object (or add a listener), and a line to reset the value..

That kind of boilerplate should never happen in code, so I usually try to find a way to drive it with data, binding the controls to objects dynamically--and towards that end, a descriptive string-based language would be very helpful.

Bill K
Your suggestion definitely has merit. Could you give a short example of a data-driven implementation? For "string", are you implying something like a domain-specific language (DSL)?
David