Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use consistent naming for dotprops #1091

Open
cafour opened this issue Aug 2, 2021 · 0 comments
Open

Use consistent naming for dotprops #1091

cafour opened this issue Aug 2, 2021 · 0 comments

Comments

@cafour
Copy link
Contributor

cafour commented Aug 2, 2021

We should name dotproperties more consistently.

Current state

The names we currently have use one or more of the following conventions:

Convention Example
Noun TextBox.Text
Noun with Template suffix Repeater.ItemTemplate
Noun with Binding suffix CheckableControlBase.ItemKeyBinding
Question Button.IsSubmitButton
Verb in imperative Repeater.RenderWrapperTag
Verb in simple past RadioButton.Checked
Collection of nouns CheckBox.CheckedItems
Adjective HtmlGenericControl.Visible

And the properties we have fall into one of the following categories according to their function:

Property category Examples
Input/Output data TextBox.Text, Checkbox.CheckedItems
Template Repeater.ItemTemplate, RoleView.IsMemberTemplate
Delegated binding CheckableControlBase.ItemKeyBinding
Status switch HtmlGenericControl.Visible, RouteLink.Enabled, Alert.IsDisplayed
Feature switch PostBack.Update, FileUpload.AllowMultipleFiles, Button.IsSubmitButton
Event command ButtonBase.Click, CheckableControlBase.Checked
Load command ComboBox.LoadItems (BP), TreeView.LoadChildren

Ideally, we'd have exactly one naming convention for each of the categories above, so that users can always infer what
the property does from its name. Naturally, in reality there are exceptions for names that we borrowed from other contexts, or names that would just be too long to be practical. We can however, try to be a little bit more consistent from now on, and maybe if we implement #420, we can even make some renames.

Proposal

I propose the following conventions:

Property category Convention
Input/Output data Nouns and collections of nouns
Template Nouns with the Template suffix
Delegated binding Nouns with the Binding suffix
Status switch Questions (with the Is, Should or Has prefix)
Feature switch Verbs in imperative (with the Allow or Require prefix)
Event command Verbs in simple past
Load command Verbs in imperative (with the Load prefix)

Note: Status and feature switches vary in bindability. Status switches are usually bindable and may change on the client (e.g. Enabled and Visible), whereas feature switches are not bindable and usually trigger a feature of some sort (e.g. RenderWrapperTag and PostBack.Update).

Problems

Switches vs Commands

We use the verbs in the past simple tense for both switches (boolean props) and commands. It can be confusing to determine which is which like in the case of RadioButton.Checked vs CheckedControlBase.Changed.

The Binding suffix

We should decide what the rule for the Binding suffix actually is. Is it for:

  • Properties of type IBinding or derived types?
    • No, because most ICommandBinding props don't adhere to this.
  • Properties of type IValueBinding or derived types?
    • No, because some ICommandBinding props don't adhere to this (e.g. GridViewTextColumn.ChangedBinding).
  • Properties of type IBinding or derived types where a DataContext change is involved?
    • No, because for example GridViewTextColumn properties don't involve a context change.
  • Properties of type IBinding or derived types where a DataContext change or property delegation is involved?
    • Yes. This seems to be the rule.

The Binding suffix therefore seems a little pointless to me since it actually says very little about what happens to the bound value. It is essentially just a nod to the user that something might happen to the DataContext unless it is just delegated somewhere else internally, which the user might not even care about (looking at you, GridViewTextColumn (#469)).

Maybe we should abandon the Binding suffix entirely and invent a different way of making DataContext changes and property delegation obvious.

Offenders

There are some offenders to the proposed conventions that we likely shouldn't rename since we're just far too used to them:

Current name Proposed name
*.Click *.Clicked
*.Enabled *.IsEnabled
*.Visible *.IsVisible
*.IncludeInPage *.IsIncluded[InPage]
PostBack.Update PostBack.RequireUpdate

There are however other changes we can make that would make the naming scheme more consistent:

Current name Proposed name
CheckBox.Checked CheckBox.IsChecked
ClaimView.HasClaimTemplate ClaimView.ClaimTemplate / ClaimView.TrueTemplate
ClaimView.HasNotClaimTemplate ClaimView.NotClaimTemplate ClaimView.FalseTemplate
ClaimView.HideForAnonymousUsers ClaimView.AllowForAnonymousUsers
*.RenderWrapperTag *.RequireWrapperTag
DataPager.HideWhenOnlyOnePage DataPager.RequireWhenOnlyOnePage
DataPager.RenderLinkForCurrentPage DataPager.RequireLinkForCurrentPage
EnvironmentView.IsEnvironmentTemplate EnvironmentView.EnvironmentTemplate / EnvironmentView.TrueTemplate
EnvironmentView.IsNotEnvironmentTemplate EnvironmentView.NotEnvironmentTemplate / EnvironmentView.FalseTemplate
GridView.InlineEditing GridView.AllowInlineEditing
GridView.ShowHeaderWhenNoData GridView.RequireHeaderWhenNoData
Literal.RenderSpanElement Literal.RequireSpanElement
RoleView.HideForAnonymousUsers RoleView.RequireForAnonymousUsers
RoleView.IsMemberTemplate RoleView.MemberTemplate / RoleView.TrueTemplate
RoleView.IsNotMemberTemplate RoleView.NotMemberTemplate / RoleView.FalseTemplate
SpaContentPlaceHolder.UseHistoryApi SpaContentPlaceHolder.AllowHistoryApi
SuppressPostBackHandler.Suppress SuppressPostBackHandler.IsSuppressed
ColumnUtils.ColumnVisible ColumnUtils.IsColumnVisible
TextBox.SelectAllOnFocus TextBox.RequireSelectAllOnFocus
TextBox.TextInput TextBox.TextInputted (?)
TextBox.UpdateTextAfterKeyDown TextBox.RequireUpdateOnKeyDown
TextBox.UpdateTextOnInput TextBox.RequireUpdateOnInput
*.HideWhenValid *.IsVisibleWhenValid
ValidationSummary.IncludeErrorsFromChildren ValidationSummary.AllowErrorsFromChildren
ValidationSummary.IncludeErrorsFromTarget ValidationSummary.AllowErrorsFromTarget
Validator.SetToolTipText Validator.AllowTooltipText
Validator.ShowErrorMessageText Validator.AllowErrorMessageText

Similar changes would apply to Business Pack and Bootstrap.


Do you agree with the proposed conventions? Did I miss something? What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant