What is better and why? (From interface-design point of view) :
a) To have two Show() and Hide() functions
b) To have one SetVisible(bool visible) function
EDIT: For example some object have visibility state and this functions is used to change it.
c) To have all three Show(), Hide(), SetVisible(bool visible) functions
I prefer SetVisible(bool visible), because it lets me write client code like this:
SetVisible(DetermineIfItShouldBeVisible());
instead of having to write
if (DetermineIfItShouldBeVisible()) {
Show();
} else {
Hide();
}
The SetVisible approach may also allow for easier implementation. For example, if a particular concrete class simply delegates the method to its composite classes, then SetVisible means one less method to implement.
void ButtonWithALabel::SetVisible(bool visible) {
myButton.SetVisible(visible);
myLabel.SetVisible(visible);
}
I disagree with all posters suggesting multiple functions to do the same thing is a good thing. Whilst three functions instead of one may not seem like much bloat, remember that your class is likely to end up with many such functions (e.g. setEnabled, enable, disable) and thus this approach will end up with a much larger class interface. Moreover, it's likely that you'll end up with a bunch of similar sounding functions/properties/whatever in your class and the multiplication of functions will further obscure which one goes with what.
In languages which support properties, these should be preferred but since neither Java nor C++ do, I guess that's a moot point.
I think setVisible() should be preferred for these reasons:
setVisible(false) you call setVisible(true) whereas the opposite of hide() could easily be reveal().setVisible(wantToSee) rather than using an if statement.setX() format generalises so you can have a set of consistent functions whereas the verbed approach spawns a host of functions that can be difficult to locate if you don't know what you're looking for. Consistency in APIs makes them considerably easier to learn and remember.It depends on what showing and hiding means in the context. First you want to figure out which one is your "main way", and focus on developing that:
setVisible(bool)
show() and hide()
OK, so now that you've coded the "gold standard" core of it, you need to figure out whether it it worth adding thin convenience-methods in the other style, to make life easier for whomever is going to use your object.
setVisible(bool)
setVisible(a==b))show() and hide()
onSuccess(widget.show))TLDR: Find out which one is most important, implement it, and then ask yourself whether it's worth adding the other style as thin convenience methods.
I would say "all three".
Show() and Hide() tend to be easier to grok than SetVisible(true) and SetVisible(false). However, when you want to set visibility logically it is better to have a method taking a bool rather than constructing an if around that bool.
You can support all three without duplicating logic and minimal boilerplate:
void Show() {
foo.Show();
bar.Show();
}
void Hide() {
foo.Hide();
bar.Hide();
}
void SetVisible(bool visible) {
if (visible) {
Show();
} else {
Hide();
}
}
Alternatively, if the things you are wrapping have a more SetVisible-ish API:
void Show() {
SetVisible(true);
}
void Hide() {
SetVisible(false);
}
void SetVisible(bool visible) {
foo.SetVisible(visible);
bar.SetVisible(visible);
}
I prefer show() and hide(), in fact, every method that receive one boolean can be changed for two methods tan express better the intention of the API. For example Robert Martin in clean code recommend prefer methods wit zero arguments over methods with one argument.
Another important argument for me is readability, in my opinion good code can be read like prose, its really strange prose something like "main_window setVisible false" instead of "main_window hide", you write or talk like that normally?, why use this strange language construction in software programs when is perfectly possible use a more natural language?.
I do believe that the more the method is expressive, the more readable, and consequently, maintainable the code will be. Consider the following two cases:
Case 1:
void showCustomerData(customerId){
Customer customer = getCustomer(CustomerId);
customerPanel.setVisible(customer.isCustomerEnabled());
}
Case 2:
void showCustomerData(customerId){
Customer customer = getCustomer(CustomerId);
//always show customer panel
customerPanel.setVisible(true);
}
In the first case, it is clear what the function "setVisible" is doing, but if you want to read it, you would say:
set the customer panel to visible if the customer is enabled or set it to hidden if the customer is disabled.
While its more descriptive to say:
- check the status of the customer:
- if the customer is enabled then show the customer's panel
- otherwise, hide it
which will change the "Case 1" function into the following:
void showCustomerData(customerId){
Customer customer = getCustomer(CustomerId);
if(customer.isCustomerEnabled()){
customerPanel.Show();
}
else{
customerPanel.Hide();
}
}
it produces more code, but is more readable.
The second case has an obvious flaw, which is, you already know that you want to show the panel, so why not use the "Show" function ?
I am not saying that using "setVisible" is absolutely wrong, but it gets confusing when you try to read code not written by you over time, and it does not conform with the "A function should perform only one operation" rule.
I believe that the Hide()/Show() alternative is attractive because it's easier to understand what's going on than with SetVisible(true), while having a single function is preferable because it avoids lots of conditionals.
If that's the case then I suggest using an enumeration as the input to SetVisible, so you get either SetVisible(Visibility.Visible) or SetVisible(Visibility.Hidden). You have a single function you can instantly read what action is being taken.
Using Java's naming conventions, you would have maybe setVisible(Visibility.VISIBLE) or setVisible(Visibility.HIDDEN).
I agree with Darien's answer, but wanted to add a point of view from a C# programmers perspective.
When I see code which says 'setXXX' I read that to say that it's setting a value on a thing, I don't expect this to have side effects in that thing other than setting that value, and I do expect this to be idempotent (ie I can keep setting it with the same value and it's okay). It's rather like accessing a field. Generally I'd also expect to see a 'getXXX' method along with a 'setXXX'.
I don't know if this is what you expect in Java and C++, but that's what I'd expect in C#, albeit in C# there's a short hand for this called Properties. And here's some nice guidance on how to use Properties (http://msdn.microsoft.com/en-us/library/ms182181.aspx).
Given this view, then the interface I'd choose depends purely on if there are any side effects (other than changing that field value):
If performing the action has side effects, for example it's showing a dialog box then I'd go with "Show()", and "Hide()".
If it doesn't have side effects, say I'm setting the visibility of a "widget" and something else renders that widget depending on it's state then I'd use setVisibility or setIsVisible. (I wouldn't call it SetVisible).
In C# (not sure about Java) it's pretty common to adopt an observer pattern, where a UI framework will listen for changes to objects and will automatically re-render the UI when a property such as Visibility changes. That means that setting the value by calling setIsVisible appears as if it has side effects, but in my definition it doesn't. The widget's contract is fulfilled by setting it's field value representing "IsVisible".
Put another way, it's okay for me to toggle the visibility of a label on a form before the form is shown. Ie label.getIsVisible == true, but the form isn't shown.
It's not okay for me to call Hide() when the form isn't being shown.
I'd suggest a slightly modified interface:
Show();
Hide();
ToggleVisible();
ToggleVisible(bool visible);
These method names help the developer to decide which method to use based on what needs to be done. Whereas SetVisible(bool visible) can confuse a developer because it conveys the same semantic meaning as Show() and Hide(), Toggle() implies the existence of a condition that determines the action. It thus becomes intuitive to the developer when to use each method.
The benefit to having multiple methods in your interface is that it simplifies the calling code. You could just expose Show() and Hide(), but:
SetVisible() private method to do the real work behind the scenes (or write redundant code for Show() and Hide()).SetVisible() (or Toggle()) already does in order to avoid code bloat (I hate redundant code). Thus duplicating a method that probably already exists as a private method in the implementation.
I would suggest using SetVisible(bool) if any only if toggling the visibility twice (show and re-hide, or hide and re-show) would leave things in the essentially the same state as before the operation was performed (it's fine if showing and re-hiding something or vice versa leaves objects in need of a redraw, provided that can be expected to happen "automatically"). If hiding and showing an object will have no effect other than changing one bit of state, then it would make sense to outside code to have some methods which accept a visibility parameter, and the writing of such code will be facilitated by SetVisible.
If hiding and re-showing an object might have side-effects such as changing the Z order, such actions should more likely be performed by separate methods. In such cases, the usefulness of outside methods which accept a "visibility" parameter will be limited, and so there will be little advantage to facilitating them. Further, a SetVisible method will (wrongly) suggest that changes to the visibility of objects can be accomplished without side-effects.
