Suppose we have the following classes:

Uml Diagram

As you see, it's an implementation of the State Pattern.

Basically, the Order class proxies the pay() and cancel() method calls to it's concrete OrderState instance.

After executing their part of business logic, the PendingOrderState and PaidOrderState call setState on the Order instance to switch to other state.

Question(s):

  1. Is it a code smell, that the public API of the Order class is "polluted" by setState, a method which should only be used internally by the OrderState instances?

  2. If the answer to the above question is "Yes", then how can the setState be encapsulated away from other clients of the Order class (like a hypothetical Customer class, who is only concerned with pay and cancel methods of the Order)?

Possible Solution:

A possible solution that comes to my mind is introducing some kind of OrderWrapper class, that would wrap the Order class and only expose the pay() and cancel() methods.

P.S

Sorry for the sloppy attempt at UML :).

up vote 1 down vote accepted

Yes, it is a code smell that a "internal" function like setState gets exposed on the public interface of Order.

What you can do about it depends to a large extent on the design of your classes and the facilities that your implementation language provides.

In a language like Java, if Order and the concrete OrderState implementations belong to the same package, then you could give setState the "package private" access specifier, making it clear that only members of the same package are allowed to use that method.

In C++, you could trade the code smell of a too large public API for the code smell of specifying the concrete 'OrderState' implementations as friends of 'Order'. That would make it possible to make setState private, but Order effectively has to know about all states that it can be in, and those state implementations have a very broad access to Order. You would have to choose which solution smells the least to you.

In other languages, there might not be a way to remove the smell and you will have to live with it.

  • Unfortunately non of these methods work for me, as my codebase is written in Typescript. Thanks for confirming that it's a code smell, though. – Alexander Lomia Nov 30 at 12:12

Your Answer

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Not the answer you're looking for? Browse other questions tagged or ask your own question.