17 votes

You are Promoted, You are Promoted, We All are Promoted!​

...
Mathieu Guindon's user avatar
11 votes
Accepted

You are Promoted, You are Promoted, We All are Promoted!​

...
Heslacher's user avatar
  • 50.4k
10 votes

You are Promoted, You are Promoted, We All are Promoted!​

You don't check your entry parameters for null. In FindSelection, this query could be reduced : ...
IEatBagels's user avatar
  • 12.4k
7 votes
Accepted

Add Default Rubberduck VBA Folder Annotation to VBProject.VBComponents

HasFolder = InStr(.Lines(1, .CountOfLines), Chr(39) & "@Folder") That condition can technically be True in ...
Mathieu Guindon's user avatar
6 votes
Accepted

Unit Testing the Duck

I don't believe a Semaphore is needed in the first place. Remove it, register an event handler and continue your "act" and "assert" phase inside it. Something like this: ...
Jeroen Vannevel's user avatar
6 votes
Accepted

Exporting a UserForm from the VBE... and working around API bugs

The logic flaw and bad assumptions were already pointed out, so how about a general review? It's time for a switch. The old code had only two cases. "If doc type ...
RubberDuck's user avatar
  • 30.8k
6 votes

Exporting a UserForm from the VBE... and working around API bugs

Well, here's a form saved from Excel 2013. Notice the VB_Exposed, VB_HelpID and VB_Description attributes. I set the Description and Help attributes using the Object Browser Context Menu ("Properties"...
ThunderFrame's user avatar
  • 1,959
6 votes
Accepted

Rubberduck Test Module Creation

Only targeting _moduleInit and _methodInit IMO the use of string.Concat with many ...
Heslacher's user avatar
  • 50.4k
6 votes
Accepted

Implementing a class hierarchy and instancing the base class in VBA

If Not DebugMode Then Resume CleanExit Else: Stop: Resume Avoid using the : instruction separator in code. That's good for ...
Mathieu Guindon's user avatar
5 votes

Evaluating Parser State

I don't know whether I've missed a nuance of this, but if you only consider the distinct states in the system the code gets a lot easier. This is typed into my browser and unchecked but I think it ...
RobH's user avatar
  • 16.9k
5 votes
Accepted

Evaluating Parser State

If I understand the rules correctly, the last rule "Otherwise return the state of the most advanced non-ready module." isn't met by your method. Having states ...
Heslacher's user avatar
  • 50.4k
5 votes

Unit Testing the Duck

There's a fair amount of setup and teardown code. I'd change many of the local variables into fields, and then add a [TestInitialize] and ...
RubberDuck's user avatar
  • 30.8k
5 votes

Unit Testing the Duck

IMO this eventhandler ...
Heslacher's user avatar
  • 50.4k
5 votes
Accepted

Code Explorer 2.0: A folder hierarchy to organize VBA projects

Looks pretty good. Some readability improvements: Magic character '.' should be moved into a named constant like ...
ChrisWue's user avatar
  • 20.3k
5 votes
Accepted

Code Explorer Commands

CodeExplorer_ImportCommand ...
RubberDuck's user avatar
  • 30.8k
5 votes
Accepted

Code Explorer UI

I don't like this: ...
Mathieu Guindon's user avatar
5 votes

Fighting f̶i̶r̶e̶ regex with f̶i̶r̶e̶ regex

Validation A public method should validate its method parameters. It doesn't matter if it is only used inside one project like RubberDuck or if it is used by other ...
Heslacher's user avatar
  • 50.4k
5 votes
Accepted

Fighting f̶i̶r̶e̶ regex with f̶i̶r̶e̶ regex

Ridiculously Large Bug Okay this is a tiny bit embarrassing, but apparently I'm an idiot. Remember when I said I recently had this in CS class? Well apparently I hadn't been paying attention. In the ...
Vogel612's user avatar
  • 25.2k
5 votes

Tick. Tick. *breathe* BOOM! - Setting up real, stable asynchronous callbacks with WinAPI Timers in VBA

First I'd like to say that this is impressive work, overall pretty squeaky clean... despite the adjustments needed to make it build on x64 :) One enhancement I can see in terms of readability, would ...
Mathieu Guindon's user avatar
5 votes
Accepted

OOP Dependency inversion principle VBA

An observation on your class constructors. At the moment you are using public properties to allow you to set the value of properties of a class after it has been created. You can take this to the ...
Freeflow's user avatar
  • 645
5 votes

Rigorously checking whether 2 floating point numbers are close in VBA

I admire the fact that you took the time to research and implement the edge cases for NaN and +-Inf. I think most people would just write an On Error Resume Next ...
Cristian Buse's user avatar
5 votes
Accepted

Produce date range string from a database table

Regarding the points of interest you identified: Using either one or two queries to get your data can be acceptable so long as it meets your performance criteria in actual use. However, I am going ...
BZngr's user avatar
  • 1,217
4 votes

Unit Testing the Duck

parser.OnParseRequested(); That's the asynchronous hook that the application uses to trigger a complete parsing of everything in the VBE: using it for your test ...
Mathieu Guindon's user avatar
4 votes
Accepted

Move Closer To Usage

There's not a lot to be said in my opinion. You've done a good job of using variables where they help clarify the intent and everything seems to be well named. The code is easy to read, even if some ...
RubberDuck's user avatar
  • 30.8k
4 votes
Accepted

Unit Test Settings

In this answer I would focus on only one area, and that is one vs multiple asserts in any given test method. The normal design suggestion is to follow the pattern of Act, Arrange and Assert, and that ...
holroy's user avatar
  • 11.5k
4 votes
Accepted

Wrapping COM objects with IDisposable

There is two things that caught my attention. Every public method / property of all implementing classes needs to call ThrowIfDisposed(). Always. That's the ...
Vogel612's user avatar
  • 25.2k
4 votes

Wrapping COM objects with IDisposable

Just a flying review: Have you tested this? The equality code isn't at all right. Firstly, never override == and != without ...
RobH's user avatar
  • 16.9k
4 votes

Constructor-injecting a truckload of commands

After rethinking the problem, I got another idea. Because it is fundamentally different, I'll post it in a separate answer. The commands are injected to the view model, exposed by properties and ...
JanDotNet's user avatar
  • 8,538

Only top scored, non community-wiki answers of a minimum length are eligible