Rubberduck: `Refactor|Rename` should rename `Property Get\Let`

Created on 23 Apr 2020  路  4Comments  路  Source: rubberduck-vba/Rubberduck

Justification
When renaming property backing fields, especially those encapsulated in a Private Type within the class, one normally wants to also rename the properties that access the backing field and vice-versa.

Description
Starting with this code:

Private Type TFoo
  MyFoo as Long
End Type
Private this as TFoo

Public Property Get MyFoo() as Long
  Foo = this.MyFoo
End Property
Public Property Let MyFoo(ByVal value as Long)
  this.MyFoo = value
End Property

Renaming MyFoo to Bar yields:

Private Type TFoo
  Bar as Long
End Type
Private this as TFoo

Public Property Get MyFoo() as Long
  Foo = this.Bar
End Property
Public Property Let MyFoo(ByVal value as Long)
  this.Bar = value
End Property

While that's exactly correct, it would be nice/incredibly handy if the code were able to recognize (hello CPA!) that this was a backing field for a Public Property and offer to rename the property as well so the end result would be:

Private Type TFoo
  Bar as Long
End Type
Private this as TFoo

Public Property Get Bar() as Long
  Bar = this.Bar
End Property
Public Property Let Bar(ByVal value as Long)
  this.Bar = value
End Property

Additional context
Once can get to the desired end by using a 2-step process of renaming the backing variable followed by renaming the property, but offering to rename both in one shot would simplify workflow and speed things up, especially for those with large projects that still have a lot of inspections to be resolved.

enhancement feature-refactorings refactoring-rename

Most helpful comment

@BZngr agreed, needs to work both ways (from the UDT or either Property member) - I'd want a prompt for the user to confirm anyway (short of a preview dialog that lets the user pick which actions are going to be carried by the refactoring).

All 4 comments

No need for CPA here - if there's a Property procedure in the same module using the same identifier name with a reference to the target UDT member inside it, then I'd think it's safe to assume the property is using the UDT member as its backing field, and prompt the user if they'd like to rename the same-name property as well.

I like this enhancement...though it seems like a special case of all the names aligning...and only in the case where the selected target is the Property. If the Property is selected, then I think renaming the UDT Member (given the name matching criteria) is safe. But, the other way around (the UDTMember is the rename target) does not _guarantee_ that it is safe to rename the same-named property (e.g., the UDTMember is referenced in another property).

I definitely see your point, @BZngr. I'm good with it working either way, though I'm probably more likely to see the backing field and want to rename it, expecting the property names to change too, than I am to remember to go to the property and make the change there remembering that the backing fields will follow. I'd learn. Eventually.

@BZngr agreed, needs to work both ways (from the UDT or either Property member) - I'd want a prompt for the user to confirm anyway (short of a preview dialog that lets the user pick which actions are going to be carried by the refactoring).

Was this page helpful?
0 / 5 - 0 ratings