64

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

user3123061
  • 1,609
  • 1
  • 15
  • 17
  • 4
    In what context? Generally it doesn't matter – Aviv Cohn Mar 20 '14 at 16:01
  • http://meta.programmers.stackexchange.com/questions/6483/why-was-my-question-closed-or-down-voted/6491#6491 – gnat Mar 20 '14 at 16:03
  • 6
    Why not have them all public? There are cases where you know they will always be shown or hidden and there are cases where you would want to conditionally show or hide. – pllee Mar 20 '14 at 16:14
  • @pllee: Thats probably very good point. – user3123061 Mar 20 '14 at 16:17
  • 4
    In java, it would be setVisible, hide, and show without the starting uppercase letter. – Pierre Arlaud Mar 21 '14 at 10:33
  • The show/hide pattern is more readable by humans, but worse to use as @Josh Kelley say. The question is whether you prefer to write or read the code. – kbec Mar 21 '14 at 17:18

10 Answers10

85

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);
}
Josh Kelley
  • 10,991
  • 7
  • 38
  • 50
  • 25
    Equivalently, you could also make Visible a property, assuming your language supports it. `MyObject.Visible = false;` strikes me as a even more intuitive than `MyObject.SetVisible(false);` – Brian Mar 20 '14 at 19:30
  • 10
    @Brian For me it's less readable and debuggable, because it hides the program behavior to my eyes - the underlying method call - but that's another story. Java does not support that syntax, anyway it's a matter of preference and eye-training. – ignis Mar 20 '14 at 22:08
  • 10
    `SetVisible()` doesn't suggest (to me) that you are actually displaying anything. It reads more like you're setting the visibility property of an object, possibly leaving it up to a corresponding `Refresh()` or `Redisplay()` method to check the value of this property to determine if the object should be displayed or hidden. – TMN Mar 21 '14 at 17:33
  • 1
    Unfortunately Java does not support properties like C# does, only getters and setters that you see above. – theGreenCabbage Mar 21 '14 at 18:15
  • 1
    @TMN: I would expect that in the absence of other factors preventing visibility (Z order, parent visibility, location, etc.) `setVisible(true)` would set in motion a process where the object would get drawn when the system was next idle, if not before. I would expect that `refresh` might be useful to hasten the display of the object, but that the object would eventually get drawn regardless (unless e.g. its visibility was set to `false` before that occurred). – supercat Mar 23 '14 at 15:55
38

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:

  1. It's immediately obvious what the inverse function is. To reverse setVisible(false) you call setVisible(true) whereas the opposite of hide() could easily be reveal().
  2. It's programmatically simpler whenever you're determining which state it should take in code, i.e. you can call setVisible(wantToSee) rather than using an if statement.
  3. Once you have multiple similar functions the 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.
Jack Aidley
  • 2,954
  • 1
  • 15
  • 18
  • 3
    C++ does not have properties, but it does have free functions, so you can extend the class interface without adding new member functions, i.e. with a lower degree of coupling. – phresnel Mar 21 '14 at 07:08
  • Qt often provides all three as a convenience so that hide() and show() can be connected directly to other events using the signal/slot system. This is really a limitation of the slot system though — if it were using something more like boost::functions, the true/false argument could be bound at the point of setting up the callback. –  Mar 22 '14 at 10:08
  • 1
    "In languages which support properties, these should be preferred but since neither Java nor C++ do, I guess that's a moot point." not necessarily. In preference to getters/setters? Yes. But set_visible isn't actually a setter. – Miles Rout Mar 28 '14 at 23:22
  • "the multiplication of functions will further obscure which one goes with what" -> Just document your API and everything will be fine – Kiruahxh Mar 20 '20 at 16:44
  • @Kiruahxh Nice joke! Ha! – Jack Aidley Mar 20 '20 at 17:12
  • ironically that's the widely used C++ and java frameworks that have `setVisible`, `show` and `hide` methods – Swift Mar 18 '21 at 17:18
19

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:

  • Reasons to pick setVisible(bool)
    • It is just a simple bit-flip, or your object is mainly holding state
    • Your object is going to spend most of its time in a CRUD framework
    • There is a lot of easily-shared code between showing and hiding
  • Reasons to pick show() and hide()
    • There are a important side-effects or lots of logic being run, such as when the object has to check all of its containers for their visibility state, or triggers a transition animation.
    • Is it part of a domain model where expressing intent is important

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.

  • Convenience of setVisible(bool)
    • Lets you avoid if-statements that have trivial conditions and only affect visibility (ex. setVisible(a==b))
    • Can be wired into certain getter/setter frameworks, if that's somethign you expect to happen
  • Convenience of show() and hide()
    • Useful in a language with first-class functions and callbacks (ex. onSuccess(widget.show))
    • Much easier to read with stack-traces and performance profiling, since you can quickly see what the program was trying to do

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.

Darien
  • 3,453
  • 2
  • 19
  • 18
12

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);
}
Garry Shutler
  • 641
  • 4
  • 8
  • 40
    Microsoft uses this approach for to start and stop `System.Windows.Forms.Timer`. Personally, I find this to be confusing. When I see both `Show` and `SetVisible`, my first inclination is to wonder if there is some important difference between the two functions. – Brian Mar 20 '14 at 19:31
  • 1
    You can easily document them to eliminate that confusion though. I didn't as this was a simple example. – Garry Shutler Mar 20 '14 at 21:32
  • 20
    So now I need to spend X extra minutes reading the documentation before I feel comfortable using the class? Or, alternatively, I need to waste X extra minutes of time being confused (or introducing bugs)? Sure, X is pretty small for this kind of thing, but it's definitely not zero. Offering all 3 options means offering three times as many functions as is necessary, which means you spending more work documenting and I spend more work learning how to use the class. Further, it introduces one more way for different developers to be inconsistent when using your class. – Brian Mar 20 '14 at 21:46
  • 5
    This is a clear violation of the Interface Segregation principle, one of the SOLID principles. Another opinion against your approach its the one from jaroslav tulach, designer of netbeans, that insist a lot of times on providing only one way to do one thing within an API in his book practical API design. – AlfredoCasado Mar 20 '14 at 21:58
  • @AlfredoCasado I agree. What if SetVisible was *protected* though? You could access it from a subclass, but call to a given entity with this interface (such as an API) would have to be Hide/Show. – Pierre Arlaud Mar 21 '14 at 10:21
  • @ArlaudPierre Then I'd still need to use the awkward `if(...)Show();else Hide();` pattern in outside code or write a `SetVisible` helper method. – CodesInChaos Mar 21 '14 at 10:33
  • ...? Show(): Hide(); isn't the end of the world either, but I get your point. – Pierre Arlaud Mar 21 '14 at 10:36
  • Three functions means 3 times as much work searching the source code to find where visibility is set. I prefer not. – david.pfx Mar 22 '14 at 05:50
  • To my mind, `show();` and `setVisible(true);` do not imply the same behavior; if a visibility flag is just one factor that may prevent an object from appearing on screen [others could include location, z-order, parent-object visibility, etc.], I would expect `setVisible()` to *only* affect the visibility flag, while `show()` and `hide()` might affect other things as well. It's useful to have a means of affecting *just* a visibility flag, but it's also useful to have a more "aggressive" means of ensuring that something can actually be seen. For example, showing a modal dialog box should... – supercat Mar 23 '14 at 15:51
  • ...modify its Z order if necessary to ensure that it's above the parent, but calling `setVisible(true)` on it should not. – supercat Mar 23 '14 at 15:52
  • Qt uses it, and it has well designed API (https://doc.qt.io/qt-5/qwidget.html#hide). As long as you document it, developpers will understand it. Moreover, when you don't know well the framework/library, you can use autocompletion to find a method that does this. If you think to show -> it's okay, if you think to setVisible -> okay too. You won't have to google for such basic behaviour. – Kiruahxh Mar 20 '20 at 16:38
5

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?.

AlfredoCasado
  • 2,159
  • 11
  • 11
  • 1
    Isn't Assembler prose enough? – Alexander Mar 21 '14 at 07:38
  • I would expect that the sequence `it.setVisible(false); it.setVisible(true);` should not affect a control's parent's visibility, nor should it affect the control's Z order or location. By contrast, `hide(); show()`; could quite plausibly force a control's parent to be visible, move it above other controls, and confine its position to someplace that can be seen. In some cases, it's useful to have a means of making sure something is actually visible (as with the aforementioned `show()`, but in other cases it's useful to change the visibility flag *without* changing anything else. – supercat Mar 23 '14 at 16:03
  • In an Object Oriented API there is no "flags", OO is about messaging, its about tell other object do some task, not about changing "flags" that are state of the object. You are making a lot of assumptions about control's, parents, z-ordering and the things that YOU expect based probably in your previous experience with other API's, its a very bad idea designing an API based on personal feelings and assumptions about a domain. – AlfredoCasado Mar 24 '14 at 11:03
5

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.

OKAN
  • 725
  • 1
  • 5
  • 8
  • I would say: `show customer panel iff the user/customer is enabled`. I do agree that there might be plenty of more complex conditions are not as easy to read as your example, however, in those cases I would split those conditions into different lines. – ComFreek Mar 21 '14 at 17:38
5

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).

Gabe
  • 375
  • 1
  • 8
3

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.

  • 1
    Your description of `getXXX()` and `setXXX()` methods as being a way to access a field without side effects sounds like Java and *not* C#. This is the way you *must* do it in Java because it does not have properties. If I saw code like that in C#, I would guess it was written by a Java developer who hadn't yet learned about properties in C#. – gilly3 Mar 22 '14 at 15:51
  • +1 for `SetVisibility`. – akaltar Mar 22 '14 at 17:44
  • @gilly3 - Yes, of course. And, "Properties" don't exist in the CLR, C# translates to method calls get_XXX and set_YYY in IL. My point is: in the context of the question, if you saw setXXX, getXXX in Java you'd expect it work with the same semantics as properties in C#. That being true, then I reason that the same guidelines for properties in C# are applicable to pairs of setXXX and getXXX in Java. I happen to agree in the guidelines I reference in the post, and I'm therefore advocating those same guidelines for use in this scenario in Java when defining the interface. – Daniel James Bryars Mar 23 '14 at 11:37
  • 1
    It might help to clarify that when you mean "side-effects", you mean "other than those associated with being an 'observable' thing". The rule I favor is to say that if a `getXX` call has a corresponding `setXX` method, then `setYY` should not affect it, but it may affect a `getZZ` call which does not have a `setZZ` method. – supercat Mar 23 '14 at 15:37
2

I'd suggest a slightly modified interface:

Show();
Hide();
ToggleVisible();
ToggleVisible(bool visible);

Better names

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.

Reduced code redundancy

The benefit to having multiple methods in your interface is that it simplifies the calling code. You could just expose Show() and Hide(), but:

  • You'd probably require some kind of SetVisible() private method to do the real work behind the scenes (or write redundant code for Show() and Hide()).
  • The calling code might have many redundant if/else blocks just to choose which method to use. This bloats the code in my opinion.
  • If I were the consumer, I'd likely just write my own wrapper function that does what 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.
gilly3
  • 169
  • 5
  • 1
    The method duplication sounds reasonable, although I wouldn't do it myself. On the other hand, I don't agree that toggleVisible(bool) is intuitive. To me, it means that it should toggle if the passed in bool is true, which would be rather strange, but I've seen stranger. It wouldn't assume that it's really a set function in disguise. – Patrick M Mar 24 '14 at 01:16
0

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.

supercat
  • 8,335
  • 22
  • 28