Should methods that are required to be executed in a specific order be private? [closed]

Closed. This question is opinion-based. It is not currently accepting answers.

---

Want to improve this question? Update the question so it can be answered with facts and citations by editing this post.

Closed 5 years ago.

Improve this question

I have a Class that retrieves some data and images does some stuff to them and them uploads them to a third party app using web services. The object needs to perform some specific steps in order. My question is should I be explicitly exposing each method publicly like so.

myObject obj = new myObject();
obj.RetrieveImages();
obj.RetrieveAssociatedData();
obj.LogIntoThirdPartyWebService();
obj.UploadStuffToWebService();

or should all of these methods be private and encapsulated in a single public method like so.

public class myObject()
{
 private void RetrieveImages(){};
 private void RetrieveAssociatedData(){};
 private void LogIn开发者_如何学JAVAtoThirdPartyWebService(){};
 private void UploadStuffToWebService(){};

 public void DoStuff()
  {
   this.RetrieveImages();
   this.RetrieveAssociatedData();
   this.LogIntoThirdPartyWebService();
   this.UploadStuffToWebService();
  }
}

which is called like so.

myObject obj = new myObject();
obj.DoStuff();

---

It depends on who knows that the methods should be called that way.

• Consumer knows: For example, if the object is a Stream, usually the consumer of the Stream decides when to Open, Read, and Close the stream. Obviously, these methods need to be public or else the object can't be used properly. (*)

• Object knows: If the object knows the order of the methods (e.g. it's a TaxForm and has to make calculations in a specific order), then those methods should be private and exposed through a single higher-level step (e.g. ComputeFederalTax will invoke CalculateDeductions, AdjustGrossIncome, and DeductStateIncome).

• If the number of steps is more than a handful, you will want to consider a Strategy instead of having the steps coupled directly into the object. Then you can change things around without mucking too much with the object or its interface.

In your specific case, it does not appear that a consumer of your object cares about anything other than a processing operation taking place. Since it doesn't need to know about the order in which those steps happen, there should be just a single public method called Process (or something to that effect).

---

(*) However, usually the object knows at least the order in which the methods can be called to prevent an invalid state, even if it doesn't know when to actually do the steps. That is, the object should know enough to prevent itself from getting into a nonsensical state; throwing some sort of exception if you try to call Close before Open is a good example of this.

---

If method B() truly cannot be called unless A() is called first, then proper design dictates that A should return some object that B requires as a parameter.

Whether this is always practical is another matter, but that's how it should be done.

---

Yes private, otherwise you are leaving the door open for users to do things wrong, which will only be a cause for pain for everyone.

Do you ever need to call any of these methods on its own? ie does any of them do anything which is useful and might be needed stand alone? if so then you might want to keep those public, but even if you keep them all public, you should have the method which calls them in the correct order (preferably with a useful name) to make things easier for your users.

---

It all depends on whether the operation is essentially atomic. In this case it looks like a single operation to us outsiders, but is it really? If LogIntoThirdPartyWebService fails, does the UI need to present a dialog box to ask the user if they want to retry? In the case where you have a single operation, retrying the LogIntoThirdPartyWebService operation also requires redoing potentially expensive operations like RetrieveImages, while making them separate enables more granular logic.

What I would do in this case is something like this:

Images images = RetrieveImages();
ImagesAndData data = RetrieveAssociatedData(images);
WebService webservice = LogIntoThirdPartyWebService();
UploadStuffToWebService(data, webservice);

or maybe more ideally something like this:

UploadStuffToWebService(RetrieveImages().RetrieveAssociatedData(),
                        LogIntoThirdPartyWebService());

Now you have granularity while enforcing the proper order of operations.

---

It sounds to me like from the consumer of your object's point of view, the object does one thing: it moves images from one place to another. As the consumer of the object, all of the individual steps you need to take to accomplish that are irrelevant to me; after all that's why I have you to do it for me.

So you should have a single DoStuff() method that takes all the necessary params, and make all the implementation details private.

---

Private -- and take the parameters in the constructor and execute the order there.

Do not assume the caller will, or knows how to, call them in order.

So, rather than the example you have listed, I would do it this way:

MyObject myObject = new MyObject(); // make a constructor to take any parameters that are required to "setup" the object per your requirements.
myObject.UploadToWebService();

---

It really depends on whether you estimate that anyone would want to invoke only one of these methods and whether they make sense individually or can be implemented independently. If not, then it is better to avoid exposing anything but the high level op.

---

Expose as little as possible, as much as necessary. If a call to FuncA() is always followed by a call to FuncB(), make one public and have it call the other, or else have public FuncC() call them in sequence.

---

Yes, it should definitely be private, especially as all the methods seem to be parameterless and you're just concerned with the order.

The only time I would consider calling each method explicitly is if they each took several, non-overlapping parameters, and you wouldn't want to pass such a long string of parameters to one method and would want to modularize. And then you should make sure to document it clearly. But remember that comments are not executable... You'll still have to trust your user a bit more than you really should.

One of the biggest factors of information hiding and OOP... only give the user what is absolutely necessary. Allow as little room for mess-up as possible.

---

The question of public or private depends entirely on the contract you wish to expose for your object. Do you want users of your object to call the methods individually, or do you want them to call a single "DoStuff" method and be done with it?

It all depends on the intended usage of the class.

In the example you've given, I'd say DoStuff should be public and the rest private.

---

Which do you think would be easier for the consumers of your class?

Absolutely write one public method that performs the correct steps in the correct order. Otherwise, the caller is not going to do it right; they're going to forget a step or skip something.

---

Neither. I think you have at least 3 objects otherwise you are breaking the Single-Responsibility Principal. You need an object that "Gets and holds images", one that "manipulates images", and one that "manages external vendor communication".

---

One reason they would be public is if you intend the user to be able to insert logic between steps. In this case, you should impose that the functions are called in the correct order internally by keeping a really tiny state machine. If the state machine transitions in the wrong order, you have options besides just doing something wrong, such as throwing an exception.

However, an alternative design that allows them all to be remain private if the case of needing to act beween steps does exist. Instead of making the methods public, provide a public callback interface that lets the users attach handlers that you call at each step of the process. In your now private doItAll() method, you can do something as granular as:

if(preRetrieveHandlerExists){
  preRetrieveHandler()
}
obj.RetrieveImages();

if(postRetrieveHandlerExists){
  postRetrieveHandler()
}
//so on and so forth

---

My software engineering rule of thumb is to always give the user/consumer/caller as little chance to screw things up as possible. Therefore, keep the methods private to ensure working order.

---

Fowler uses the term "Feature Envy" to describe a situation where one object calls a handful of methods (especially repeatedly) on another.

I don't know where he got it from. You don't see it much in the literature, and a lot of people over the years have had no idea what I was talking about (I dunno why, I thought the name was perfectly obvious once I heard it. Which is why I repeat it)