DO12 - Dialog - "Control" object methods

First of all, congratulations to the new dialog editor! o) I had some first tests and played a little, it feels solid and stable. In case you did that from scratch, hats off! o) It's really fine, intuitive and get's you going without any trouble. Nothing to not like about it. o)

Then I encountered the new scripting API, which is to be used to alter and handle the dialog controls and events. The method naming and separation on the new "Control" object is something my mind stumpled upon. Assuming that "beta" does not necessarily mean things are hammered into stone, I'd like to ask if we could have a little conversation regarding this part of the newer scripting features. I'm not planing to get über-verbose, don't be afraid. o)

So, is there any open ear out there? o)

Sure, do your worst :slight_smile:

Ok, aehm.. really??? o)

Fine! o) I will come back tomorrow and continue where we left, I just need a little time to prepare and dress my thoughts. o)

It may help (and I don't know what your comments will be, of course) to point out that as the controls are standard Windows controls, the methods the Control object exposes are derived from the standard Windows control messages. For example, combo boxes have a CB_ADDSTRING message, and the AddString method exposes this message.

But I look forward to your feedback :slight_smile:

Sorry, took some more days to come back. o) The API listings below were actually done before my first post, but I did not find a calm minute to continue and add some explanations to them. Thanks for the details on why the methods are named like they are currently. From a c++/win32 programmers point of view, it maybe a nobrainer to basically just "reuse" the names for the exposed methods like you did. Just guessing, this CB_ADDSTRING etc. API is ancient? As someone who's seen a handful environments targeting user interfaces, recently building WPF applications and things, I'd like to tell: The method names on the Control object feel somewhat outdated, sometimes wrong and seem extra hard to use. It's quite far from what you are used to work with in nowadays frameworks, wether it's WPF, .Net-Forms or Web/HTML-Forms.

The following is an overview of the current method names, followed by alternative #01 and #02. The latter uses properties for some of the exposed functionality to reduce the number of methods in total. Red relates to the current naming, green to suggested alternatives. In the end you find a more detailed explanation to each current Control object method and why I think a different name or an additional method/property enhances the current API.

Current Beta-API
[ul]
[li]too short names [/li]
[li]not easy to use, unclear results[/li]
[li]hard to read [/li]
[li]hard to understand [/li]
[li]does not meet any standard (means standards "I" know at least! o)[/li]
[li]unclear what these methods accept as parameters (you'd end up with help-file open all day long)[/li]
[li]unclear what they act on
[ul]AddString()
InsertString()
DeleteString()
GetCheck()
SetCheck()
GetCount()
GetData()
SetData()
GetVisible()
SetSel()
GetText()
SetEnable()
SetVisible()
[/ul][/li][/ul]

Alternative API #1
[ul]
[li]more friendly, can almost be read fluently[/li]
[li]better separation of functionality[/li]
[li]intuitive naming, less guessing about parameters and expected result[/li]
[li]nearly "standard" in nowadays GUI toolkits[/li]
[li]clear differentiation between simple values and Items (which have name and (optional) value attached)
[ul]AddItem()
InsertItemAt()
RemoveItemAt()
RemoveItemByName()
IsChecked()
SetChecked()
Count()
GetItemValueAt()
GetItemValueByName()
SetItemValueAt()
SetItemValueByName()
IsEnabled()
IsVisible()
GetSelectedIndex()
SelectItemAt()
SelectItemByName()
GetValue()
SetValue()
Enable()
Disable()
Show()
Hide()
[/ul][/li][/ul]

Alternative API #2 (my preferred version)
[ul][li]using all the "pro" points of alternative #01[/li]
[li]making use of read/write properties -> less methods required[/li]
[li]gets around difficult naming for getters and setters of boolean values (Get..()/Set..() vs. Is/Has..()/Set..() etc.)[/li]
[li]even easier handling[/li]
[li]also known from other frameworks
[ul]AddItem()
InsertItemAt()
RemoveItemAt()
RemoveItemByName()
GetItemValueAt()
GetItemValueByName()
SetItemValueAt()
SetItemValueByName()
SelectItemByName()
.checked [instead of IsChecked() + SetChecked()]
.count [instead of Count()]
.selectedIndex [instead of GetSelectedIndex() + SelectItemAt()]
.enabled [instead of IsEnabled() + Enable() + Disable()]
.visible [instead of IsVisible() + Show() + Hide()]
.value [instead of GetValue() + SetValue())]
[/ul][/li][/ul]

Detailed explanation: (method name mapping between current beta and alternatives)
[ul]
[li]AddString( strVal [, strData] )
-> AddItem( strName, [, strValue] )
AddString() does not only add a string, it adds a string (which can be seen as a name) and a corresponding value.
A name/value pair can be seen as an Item. New methods reflect that. Notice the difference strVal -> strName, strData -> strValue.
[ul][/ul][/li]
[li]InsertString( strVal [, strData, intIndex] )
-> InsertItemAt( intIndex, strName [, strValue,])
It's about indexes here compared to AddItem()/AddString(), so index should be first parameter.
Index first would also avoid dummy "strData = 0" parameter each time (optional params always go last).
Notice the difference strVal -> strName, strData -> strValue.
[ul][/ul][/li]
[li]DeleteString( intIndex or strVal )
-> RemoveItemAt( intIndex )
-> RemoveItemByName( strName )
"Remove" is more counterpartish to "AddString()" than "DeleteString()".
Its weird how the current DeleteString() accepts an integer to remove "strings" which actually are entries or items.
[ul][/ul][/li]
[li]GetCheck()
-> IsChecked() or
-> GetStatus() and/or
-> .checked (if read/write properties are possible)
Since GetCheck() also works for radiobutton, the name is misleading.
Does it return a checkbox? It works on radiobuttons as well, so GetStatus() or something seems to fit much better.
[ul][/ul][/li]
[li]SetCheck( intStatus )
-> SetChecked( intStatus )
-> .checked (if read/write properties are possible)
Similar to GetCheck(), since SetCheck() also works for radiobutton, the name is misleading.
[ul][/ul][/li]
[li]GetCount()
-> Count()
Consider turning this into a .count property like in other DO objects, or call it Count().
Because there is .size, .length and .count, another variation like GetCount() can be avoided.
[ul][/ul][/li]
[li]GetData( intIndex or strVal)
-> GetItemValueAt( intIndex )
-> GetItemValueByName( strName )
GetData() is really hard to get. What data from where?
The alternatives make clear that it's about getting a value from an item in a specific position or with a specific name.
[ul][/ul][/li]
[li]SetData( intIndex or strVal, strData )
-> SetItemValueAt( intIndex, strData)
-> SetItemValueByName( strName, strData )
It's hard to understand as well. How much easier would "SetItemValueAt( index)" be to anyones brain?
Quite a lot I think, SetData() tells me nothing, it does not speak to me.
[ul][/ul][/li]
[li]GetEnable()
-> IsEnabled() and/or
-> .enabled (if read/write properties are possible)
GetEnable() does not sound right and makes little sense.
Does it get the control and enable it? No, it just returns a bool indicating the enabled/disabled status.
[ul][/ul][/li]
[li]GetVisible()
-> IsVisible() and/or
-> .visible (if read/write properties are possible)
Same as GetEnable().
[ul][/ul][/li]
[li]GetSel()
-> GetSelectedIndex() and/or
-> .selectedIndex (if read/write properties are possible)
Does GetSel() get you the current selection? Is it a text? No, it's about the selected index from a list or combobox.
[ul][/ul][/li]
[li]SetSel( intIndex or strVal )
-> SelectItemAt( intIndex )
-> SelectItemByName( strName )
SetSel(), same as GetSel(). SetSel() does not make clear what it does and what parameters it accepts.
"SelectItemByName()" is much more easy to understand in comparison.
[ul][/ul][/li]
[li]GetText()
-> GetValue()
-> .value (if read/write properties are possible)
[ul][/ul][/li]
[li]SetText( strVal )
-> SetValue( strVal ) and/or
-> .value (if read/write properties are possible)
[ul][/ul][/li]
[li]SetEnable()
-> Enable()
-> Disable()
-> .enabled (if read/write properties are possible)
Syntax like SetEnable(false) is hard to understand and hard to read.
[ul][/ul][/li]
[li]SetVisible()
-> Show()
-> Hide()
-> .visible (if read/write properties are possible)
A name like SetVisible() implies that it always unhides items, but it does not.[/li][/ul]

Icing on the cake:
Depending on how much object oriented you want the DO API to be. An additional list and combobox Item object could be invented. This one could be prepared, added and removed "at once", in contrast to methods which always take two parameters for name/value ("string" and "data"). A (List/Combobox-) Item object and matching methods to work with makes even more clear, when a method acts or depends on simple values (Edit/Textbox-Control e.g.) or wether it is about a name/value pair - a (List/Combobox-) Item. Quick example (using suggested API): Instead of value = GetItemValueAt(1), you'd write value = GetItemAt(1).value etc.

Thanks for reading! o)

Thanks for the detailed suggestions. We'll see what happens :wink:

We'll go with your suggestions for the next beta (and deprecate the old methods) to try it out and get some feedback.

For the most part your ideas make sense. I'm a bit dubious about the ".value" property though, since this will now have three meanings:

  1. Getting/setting the text value of an edit control
  2. Getting/setting the label of a checkbox/button/etc
  3. Getting/setting the data for the selected item in a combo or listbox

#1 makes sense but I'm not sure #2 is that logical (it's a label, not a value), and although #3 could be replaced with GetItemValueAt(GetSelectedIndex()) that seems rather clunky. Comments?

If a button/checkbox had a ".value" property, I would expect it to tell you whether or not the control is pushed/checked. (Pushed as in with a toggle button. Doesn't make much sense with a normal button, which I guess is where Tbone was headed with the if read/write properties are possible clause.)

In his suggestions though, that's what ".checked" does. Maybe it makes more sense to remove ".checked" and make ".value" do that for checkboxes/radio buttons, and to add a ".label" property that handles the label of things that have one.

But along those lines, ".value" should arguably do what ".selectedIndex" does as well.

1) right, .value was meant for edit/text/integer input controls (only)
2) uhm, actually no, .value should not set labels, if possible go for the .label property instead.
I did not look on labels and how they can be set and get it seems, but .label seems fine.
3) .value to get the value of the currently selected item is.. well I don't know, sounds good at first, but since a value/data for list/combobox controls is optional, people might want to get the name of the selected entry in the first place. So have .name as well? (getting the selected index's name is missing alltogether it seems?). Anyway - using .value and .name to interact with the currently selected item is a bit mushy, since the item behind is not clear (is it the control itself or the selected item within the control). I might choose .selectedValue and .selectedName here to separate them a bit, these match .selectedIndex quite good as well.

This is tricky without introducing a dedicated Item object. I mentioned it briefly, but with a separate object, you would not have to worry much about property names and such. myComboBox.selectedItem.value is probably as nice as it can get. .selectedItem always gives you the selected items object, .selectedIndex gives you just its index, getItemAt() an item at an index. getItemByName() an item by name. .items could be a regular collection on the control, exposing .count and whatelse is available already. A dedicated Item object could also be used to stuff some more properties in the future (think of .enabled, .(is)Separator, .fontSize, .(is)Indented etc.). But implementing this is quite some additional load of course. o)

.checked vs. .value:

  • buttons should not have .value if they can have .label, I agree
  • for checkboxes/radios .checked still seems fine to me, indicating it's a boolean to set and get. The tri-state checkbox is an exception to this and the only reason to go for .value here as well, but if not, .value could be reserved for "real" mixed-type values like objects and strings/ints.

Ok, this is what I've decided to go with. Hopefully meets your approval :slight_smile:

Unfortunately it proved too hard to continue to support the old methods so if anyone has actually used the script dialog stuff yet you may need to make a few changes to your scripts after the next beta.

Control object:

AddItem(strName [, strData])
InsertItemAt(intIndex, strName [, strData])
GetItemAt(intIndex)
GetItemByName(strName)
RemoveItem(intIndex or objItem)
SelectItem(intIndex or objItem)
.count (readonly)
.enabled (r/w)
.visible (r/w)
.label (r/w)
.value (r/w)

on GET, .value will return a string (edit field), a bool (simple checkbox/radio button), a long (tri-state checkbox or tab), or a DialogListItem (combo/listbox)
on SET, .value takes a string (edit field or tab), a bool or long (checkbox/radio/tab), a long or a DialogListItem (combo/listbox)

The DialogListItem has the following properties:

.index (readonly)
.data (r/w)
.label (r/w)

Hi Jon, this looks promising! o) Few questions and remarks though. o)

  1. These two methods below won't accept DialogListItems?
    Now that DialogListItems have been born, these two methods should definitely eat those objects I think.
    AddItem(strName [, strData])
    InsertItemAt(intIndex, strName [, strData])

  2. How do you determine the currently selected item/index?

  3. DialogListItem features a .data property, why not call it .value here as well?

  4. GetItemByName( strName ) get's you a DialogListItem by label I guess, so it should be called GetItemByLabel( strLabel )?

GetItemAt() and
GetItemByName/Label() look good! o)

I miss..
RemoveItemAt()
RemoveItemByLabel() and
SelectItemAt()
SelectItemByLabel() though. o)

I don't know if it makes sense to mix accepted parameters and naming on the methods responsible for adding/removing/selecting DialogListItems. Also don't know if it makes sense to be able to remove items by the original or same object. It makes sense to create a new DialogListItem for adding (which you skipped), but it does not seem practical to create new DialogListItems or keep references to items that shall be removed or selected. Two ways to solve this, split and change existing RemoveItem()/SelectItem() into the four methods listed above or combine GetItemAt() and GetItemByLabel() into GetItem(), which accepts ints for "by index", strings for "by label" and obj for "by object".

So it should read..
GetItemAt( intIndex )
GetItemByLabel( strLabel)
RemoveItemAt( intIndex)
RemoveItemByLabel( strLabel )
SelectItemAt( intIndex)
SelectItemByLabel( stLabel)

or..

GetItem( intIndex or strLabel or oListItem )
RemoveItem( intIndex or strLabel or oListItem )
SelectItem( intIndex or strLabel or oListItem )

to end up with consistent naming, params and intuitive usage.
It looks like you went for supporting objects at one end, while taking another approach elsewhere, resulting in quite a mixed handling.

The properties look very good though, nothing questionable on them from my point of view! o)
Thank you! o)

ps: Don't know how return values look like, but I'd think it would be cool if RemoveItem..() and SelectItem..() return the affected DialogListItems on success, just like GetItem..() does. Well, I guess it does. o)

Good point about GetItemByName, will change that to Label.

I can't see any point in AddItem/InsertItemAt taking an item object since these don't exist except when items have already been added to a list. Unless you wanted to copy an item from one list to another, and even then you can just do AddItem(item.label, item.data).

Currently selected item index comes from .value property.

But look, creating an object, setting it's properties and passing it on to something that attaches and uses it somewhere is common practice (think of handling xml nodes and attributes in the MSXML doc, html elements in browsers etc.). Not using an object in the add/insertion process destroys the whole DialogListItem object "thinking".

If Add/InsertItem() could take an object

  • this part of the API can stay as it is in case you add more features to the list item object or the control they reside in (speaking of font, size, disabled, separator-type and the things I already mentioned). What happens in case you add .font to DialogListItem? The api'd be totally screwed.
    You'd need to add items by Add/Insert(), then get it back with Get(), set the .font and push it back to the list, just weird in comparison to setting .font directly on what comes out of "Control.CreateListItem(strLabel, strValue)" and calling Add(myListItem).
  • you give reason for DialogListItem to exist, it's irritating and not consistent if you can only use it for Remove() and Select() and who'd ever make use of DialogListItem in these cases if you need to Get() it by index/value first? Probably noone, since using the index here is way easier and straightforward.
  • you mentioned it, you can copy/move items from one list to another
  • you can create, mix and handle items before they are added without using artifical things to hold possible DialogListItem data.
  • you reach consistency in the api, it's very unusual for a set (related) methods to use an object only sometimes and sometimes not

Well, using .selectedIndex would be what you are looking for if you are new to this environment and experienced with other GUI frameworks. Using .value here also might suggest that you get the value of the currently selected item or something. I know you are fond of short prop-names, aren't you? o) But seriously, .value isn't the best choice here.

My feelings right now are like this:
If you don't want to support DialogListItem handling in add/creation step.
Remove it from RemoveItem() and SelectItem() as well, since object support right there does not add anything despite blurrying the concept.
It's better choice to stick to one of the method-listings not making use of DialogListItem in this case. It should be object here and everywhere or not at all. A mixed usage is questionable and I must think hard if I ever encountered it elsewhere.

Did you know? There are only 3 hard things in information technology:

  1. naming things
  2. off by 1 errors

Point 1 came to my mind, while writing this. It surely is hard. The joke just came with it. o)

ps: Regarding my previous post and the following possible variation mentioned:
GetItem( intIndex or strLabel or oListItem )
RemoveItem( intIndex or strLabel or oListItem )
SelectItem( intIndex or strLabel or oListItem )
Don't! I was tired when writing this. You can't ever be sure what happens with these methods accepting integers and strings to work on a specific item by index or label. A label can be "10" and so can an index, very easy to mess up logics. The listed, separated methods like RemoveItemAt() and RemoveItemByLabel() are far more safe to use. Add Select/RemoveItem() to take the object in case it lives on and things end up as clear as they can get.

New suggestion in case the DLI object lives on:
GetItem( objDialogListItem ) //might not make sense at first look, but does alone for testing if an item still exists
GetItemAt( intIndex )
GetItemByLabel( strLabel)
RemoveItem( objDialogListItem )
RemoveItemAt( intIndex)
RemoveItemByLabel( strLabel )
SelectItem( objDialogListItem )
SelectItemAt( intIndex)
SelectItemByLabel( stLabel)

Or with 3 specific parameters, you'd always NULL two of them to be safe in the field, not very nice and intuitive but safe:
GetItem( intIndex or NULL, strLabel or NULL, oListItem or NULL)
RemoveItem( intIndex or NULL, strLabel or NULL, oListItem or NULL)
SelectItem( intIndex or NULL, strLabel or NULL, oListItem or NULL)

The API doesn't have to be perfect, just functional :slight_smile:

No API is perfect I guess, so yes. o)
But why overhaul a set of methods, if the final result has equally strange aspects as before?

As said, introducing DLI does not add much at this stage, the concept behind should be finished, so it actually makes sense. I'm also fine with dropping DLI again, since it would keep things simple, lean and still intuitive. The current mixture does not feel good and it also does not add functionality. It adds new aspects to take care of, so it's harder to use in the current stage in comparison to the finished DLI concept or the KISS variant without.

AddItem and InsertItemAt will accept DialogListItem objects now.

The right decision! The extra mile you went here was worth it, I'm quite sure it'll show very soon. o)


May I ask, how do you actually create these list items now?