VBA Rubberducking

Duga

12:00 AM

RELOAD!

[Minesweeper] Games Played: 93, Bombs Used: 57, Moves Performed: 12531, New Users: 13

RELOAD!

[Zomis/minesweeper-server] 1 commits. 1 additions. 1 deletions

Mathieu Guindon

@SmileyFtW ~waves

SmileyFtW

4:08 PM

@Duga this link is 404: [Zomis/minesweeper-server]

Duga

5:16 PM

[rubberduck-vba/Rubberduck] MDoerner added label PR-Status: Review Requested to pull request #5387: Split refactorings

Quack Exchange

5:41 PM

1

Q: Survey Populate with List, Option Label and Formula, VBA

I am quite a novice. The below code does what I require but I was hoping you could help me speeding it up and simplifying it please. I know should start with basics when learning VBA..but it's just so exciting. Probably it would be faster if I applied For i to x, next i method Could you please h...

Duga

5:58 PM

[rubberduck-vba/Rubberduck] retailcoder commented on Rubberduck.Refactorings/Abstract/BaseRefactoringWithSuspensionBase.cs of pull request #5387: Split refactorings

Duga

7:29 PM

[rubberduck-vba/Rubberduck] MDoerner commented on Rubberduck.Refactorings/Abstract/BaseRefactoringWithSuspensionBase.cs of pull request #5387: Split refactorings

M.Doerner

Argh, ExtractInterface does not work well with annotations.

I puts the Implements statement between the member annotations of the first member and the member.

Moreover, it does not transfer @DefaultMember attributes to the interface.

Why exactly do we put the implements statement right above the first member?

I would usually always put them right below the option statements.

this

I think that's to handle the case where there are none

M.Doerner

7:45 PM

Then I would put them at the top of the module.

Th place we put them right now is definitely problematic.

Duga

8:43 PM

[rubberduck-vba/Rubberduck] MDoerner synchronized pull request #5387: Split refactorings

[rubberduck-vba/Rubberduck] codecov[bot\] edited comment on pull request #5387: Split refactorings

> # [Codecov](https://codecov.io/gh/rubberduck-vba/Rubberduck/pull/5387?src=pr&el=h1) Report
> Merging [#5387](https://codecov.io/gh/rubberduck-vba/Rubberduck/pull/5387?src=pr&el=desc) into [next](https://codecov.io/gh/rubberduck-vba/Rubberduck/commit/dc732efef8fb3a7ced16e4af16e3257ed10fea11?src=pr&el=desc) will **increase** coverage by `0.14%`.
> The diff coverage is `87.98%`.

```diff
@@ Coverage Diff @@
## next #5387 +/- ##
=========================

Duga

8:59 PM

[rubberduck-vba/Rubberduck] build for commit d26eef58 on unknown branch: AppVeyor build succeeded

[rubberduck-vba/Rubberduck] codecov[bot\] edited comment on pull request #5387: Split refactorings

> # [Codecov](https://codecov.io/gh/rubberduck-vba/Rubberduck/pull/5387?src=pr&el=h1) Report
> Merging [#5387](https://codecov.io/gh/rubberduck-vba/Rubberduck/pull/5387?src=pr&el=desc) into [next](https://codecov.io/gh/rubberduck-vba/Rubberduck/commit/dc732efef8fb3a7ced16e4af16e3257ed10fea11?src=pr&el=desc) will **increase** coverage by `0.16%`.
> The diff coverage is `88.01%`.

```diff
@@ Coverage Diff @@
## next #5387 +/- ##
=========================

[rubberduck-vba/Rubberduck] build for commit d26eef58 on unknown branch: 61.76% (target 0%)

[rubberduck-vba/Rubberduck] build for commit d26eef58 on unknown branch: 88.01% of diff hit (target 60%)

Duga

9:21 PM

[rubberduck-vba/Rubberduck] bclothier commented on Rubberduck.Refactorings/Abstract/IBaseRefactoring.cs of pull request #5387: Split refactorings

this

I noted that for the interface refactoring, we add a Err.Raise 5, , "Not Implemented" for the interfaces. I'm just wondering why 5, though? Wouldn't it be more accurate to use the HRESULT value of E_NOTIMPL?

Duga

[rubberduck-vba/Rubberduck] MDoerner commented on Rubberduck.Refactorings/Abstract/IBaseRefactoring.cs of pull request #5387: Split refactorings

M.Doerner

I have not really looked at the implementations of the refactorings.

That said, I believe we use 5 because that is the closest thing appearing in VBA natively.

AFAIK, the HRESULT for E_NOTIMPL is not a VBA error code and does not have an error message in VBA.

this

9:37 PM

@Duga Fine but I think my point still stands - the new IRefactoring is basically just the refactoring while the original IRefactoring takes some kind of selection. To call it BaseRefactoring makes it more confusing than necessary. What about calling the original IRefactoring ITargetedRefactoring or ISelectionRefactoring?

Even IUserFacingRefactor might be better.

M.Doerner

I think in RD a refactoring is the thing encompassing all that belongs to a refactoring, i.e. it includes acquiring the target and validating it.

That is not what the ``IBaseRefactoring` does.

I pondered whether to name it IRefactoringImplementation, but that would also give the wrong impression since it is not the implementation of a refactoring.

It is just the execution part.

BTW, I am also not too happy with the name IBaseRefactoring.

this

But we have the comment saying that `IBaseRefactoring is the heart of the refactoring, the one where we actually execute the refactoring.

M.Doerner

I just could not come up with a better name.

Yes, but it is not the whole refactoring.

this

Am I to understand that the BaseRefactoring is a separate hierarchy from the Refactoring?

M.Doerner

Yes, it is.

this

9:46 PM

which make it even more harder to understand, I feel.

M.Doerner

The refactorings now use base refactorings, but are not forced to do so.

this

because when I saw the IBaseRefactoring, it looked like a trimmed down version of IRefactoring

M.Doerner

In a certain way, it is.

this

since both has a Refactor() but only the original IRefactoring has the additional methods for taking a declaration or a selection

and so I'm following, the new BaseRefactoring stuff are meant to implement what we were earlier calling "mini-refactoring"s that other refactors may use, right?

If that is correct, then a possible name would be IRefactoringAction or IRefactoringUnit

M.Doerner

That only works for implementations of ICodeOnlyBaseRefactoring.

I can live with IRefactoringAction and think that it is an improvement over IBaseRefactoring.

The problem with the other implementations is that they only provide methods that execute a rewrite.

For some refactorings, this cannot be avoided since they do things like renameing or adding components, which invalidates the state for any other refactoring.

BTW, most refactorings do not implement ICodeOnlyRefactoringBase since I was not sure whether collisions could happen.

Completely unrelated, it is really annoying to adjust the tests for the ReferenceReconciler; everything magically appears from arrange methods that have nothing to do with each other on first sight, but actually have to be configured in a matching configuration.

the VBE - it's right for me

10:31 PM

is there any way around static classes not being able to implement interfaces?

i was trying to generalize the method i used to generate an instance of the list for the other structure types i am using

but i dont know how to make it work without interfaces

and dont think it makes sense to not have the base wrappers in a static class

if code helps, here is the structure i have:

public static class DotNetListWrappers

    public interface IListWrapperBase<T>

    public class ListWrapperBase<T>

point was to have each structure wrappers defined like this, then send the static class to the general method GenerateStructure

so like in addition to DotNetListWrapper there is DotNetHashSetWrapper etc

i like the idea of have each structure wrapper in a separate code file for clarity

is there an alternative to using static or something else obvious i am missing? if it is work for someone to answer ill do more research myself but if its easy...

Transcript for