Reputation: 20834
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?
Upvotes: 4
Views: 569
Reputation: 62789
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.
Upvotes: 0
Reputation: 46475
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)
Upvotes: 1
Reputation: 47899
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?
Upvotes: -1
Reputation: 36035
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.
Upvotes: 11
Reputation: 41271
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);
Upvotes: 6
Reputation: 121414
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);
Upvotes: 8
Reputation: 125874
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.
Upvotes: 1
Reputation:
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...
Upvotes: 14