Hot answers tagged

27

OK, went through the code and gave this a bit of thought over the past couple of days. As far as the implementation goes, I don't see a whole lot that I would change (that you didn't identify in the answer above) other than a couple nit-picky things. First, is the use of the this variable identifier. I couldn't find anything that justifies the naming and ...


20

Public Function ToString() As String 'Returns a string that represents the current List object. ToString = StringFormat("{0}<{1}>", TypeName(Me), _ Coalesce(this.ItemTypeName, "Variant")) End Function This means the string representation of this List is "List<Variant>" when this.ItemTypeName is empty ...


17

This seems extra complexity with no purpose. You take any type variable and automatically convert it to a parameter (this is good). But then something strange happens, you look at the type of the variable and convert that to a string so you can call a function named after the type to do a standard set of options that only change based on the type. Why ...


15

Key Points Each Case block implements formatting functionality for a specific format specifier. Goto statements indicate the function wants to be broken down into several smaller functions. Local variables such as alignmentSpecifier, alignmentPadding, precisionString, precisionSpecifier, formatSpecifier and all others, could all be eliminated if there was a ...


15

Stop. Close Excel. Take a deep breath. Now go and get an actual Database. MS Access. MySQL is free and Open-Source. It doesn't really matter. What matters is that Excel is not the program to be doing this in. At all. 10-Dimensions was already excessive. 31 is a severe case of Over-Engineering. Unless you've been specifically required to include support ...


14

The code you have works, and is very nicely-formatted. There are only two things I would change, stylistically speaking: Explicitly pass input parameters by value. This should have been the default, but it wasn't in classic COM-based VB, so you need to write it out. ByRef is actually a perverse choice for the default, since it is almost never what you want....


12

Error Handling Your error handlers look much cleaner (and ultimately safer) than before. I also like your GetErrorSoure() routine and the "CleanExit:" name of the labels. Very concise. (<-Read as, "I'll be 'borrowing' more of your code'). I see a small issue in your OpenFile() routine. You should probably set OpenFile = False prior to re-raising the ...


12

The only coupling I can see with MSAccess-specific vba is in your exampleCall (why is it Private anyway?): Dim prj As vbProject Set prj = VBE.ActiveVBProject Your code works perfectly fine with Excel vba if you take in a VBProject parameter: Public Sub exampleCall(project As VBProject) If this code lives in a class module called Ext, I can then do this ...


12

I'll just focus on performance, because that's the fun part. You're correct to be worried about the string manipulations, so you should obviously try to minimize them. The other thing you should be worried about is the division and exponentiation used for bit-twiddling. If all you need to do is swap the byte order of an intrinsic type, you can do it using ...


11

AdoConverter For better extensibility, the methods in that class shouldn't be calling each others the way ToLongParameter is calling ToIntegerParameter. Also instead of hard-coding the type Private Type TypeMappings BooleanMap As ADODB.DataTypeEnum ByteMap As ADODB.DataTypeEnum CurrencyMap As ADODB.DataTypeEnum DateMap As ADODB.DataTypeEnum ...


11

Two small things I noticed at a glance, both involve this code and it looks like it made it into your refactored version as well. 'some error-handling constants: Const ERR_FORMAT_EXCEPTION As Long = vbObjectError Or 9001 Const ERR_ARGUMENT_NULL_EXCEPTION As Long = vbObjectError Or 9002 Const ERR_ARGUMENT_EXCEPTION As Long = vbObjectError Or ...


11

The only thing that I can see at first glance to show what the business rule is, is to do something like this m_GlobalCode = "ABC" result = GetEquivalentCode Debug.Assert result = "ABC2013" probably at the beginning. I think this is exactly what the business rule is saying from my understanding of the information that you have given you probably want ...


10

I think the itself class might be mis-named, because it is really 'Empty-able' not Nullable or 'Nothing-able'. You have to keep in mind that Empty, Null, and Nothing are very different concepts in VB6. Setting and object to Nothing is basically just syntactic sugar for releasing the pointer to the Object. This is the same as asking for ObjPtr() to return ...


10

You need to change/add just one little thing. You need to call the Randomize () method before using the Rnd () method. You should only call it once, so it would be good, to call the Randomize() method inside of the constructor of your class like: Private Sub Class_Initialize() Randomize End Sub If you don't do this you will get for each iteration the ...


10

Design I think you need to break that class in two. Having instance members on a static class is pretty confusing and bug-prone. I'd suggest Enumerable to be the static class, with these members (notice source being preferred over collectionObject, and explicit ByRef modifiers and Variant types): Function Contains(ByRef source As Variant, ByVal value As ...


10

Obviously @Comintern knocks out the bit-swapping part, so this answer is more about, well, every other aspect of the code. Signature Kudos for proper PascalCase! If the function is meant to be Public, it's probably best to specify it as such. I like that it has an explicit return type, but the parameter is implicitly passed by reference, which is ...


9

I would opt for strict type checking here. It seems a bit lazy to force it to a single when implicit in the function name. No need to use a variant and force it to a Single via a cast. IMHO, if the function ToSingleParameter is expecting a Single, then it should get a Single value and complain with a type mismatch error if it doesn't receive it. I've ...


9

You only asked for suggestions how to make this code faster. However, after discussing what might be the issue with performance here, I will provide some comments regarding general maintainability issues. Performance Generally, what you are doing in the procedure on the client side would much better be handled on the database server side as a stored ...


8

My 2 cents, the first function seems fine, you could make it a little DRYer by just setting the compareMethod in your if statement and then have only 1 complicated line of logic. And if you are doing that, you might as well put the Cstr's there. Public Function StringContains(haystack, needle, Optional ByVal caseSensitive As Boolean = False) As Boolean ...


8

Your implementation of Sort would be optimised by implementing IndexOfMin so that Min = Item(IndexOfMin) (with a caveat for an empty List), but you can then use the O(1) RemoveAt instead of the O(n) Remove at the end of the Do Until loop. Similarly for IndexOfMax, Max and SortDescending, of course. I'd consider making IndexOfMin and IndexOfMax Public to ...


8

In my opinion, tests should be like a class, they should have only one responsibility. For a test, that would mean every time you want to show a special case, you should have a test method. The name that would represent what will be return or what the method is currently testing. Is a corner case, a typical case or just some examples ? In your particular ...


8

Your code is small, clean and readable and in my eyes it is well optimised code. If you are in need of different upper and lower bounds for your random numbers I suggest you implement the following : Private Function RandomInteger(Lowerbound As Integer, Upperbound As Integer) As Integer RandomInteger = Int((Upperbound - Lowerbound + 1) * Rnd + ...


7

This code looks very clean, It looks like it handles edge cases well. I especially like the way that if it converts to the specified data type that it does it and doesn't make you convert explicitly. that is very slick! I would definitely like to see some code that puts it into play. Not much to review here, but very good code and good syntax. Edit I ...


7

Adding to @Comintern's excellent answer, the private type doesn't need an IsNull member, since the class only accepts value types, the correct semantics for "null" values is vbEmpty. The Set accessor is therefore not only wrong, it's also ambiguous - not only in attempting to assign Nothing to a value type, but also because Value being the default member, ...


7

Your GetEquivalentCode procedure is flawed. The IsMissing() function requires a variant; it will not behave as you may imagine when passed a string variable. Optional parameters if not variants will be assigned their default values: passed or not, they will be there. Your initial test cases all had this. To understand the risk here try this: Private Sub ...


7

Ok so this is what I ended up doing. I created an interface, called it ITestClass.cls: Option Explicit Public Sub RunAllTests() End Sub Then I created a new module, called Tests.bas: Private AllTests As Collection Option Explicit Private Sub SetupTestClasses() Set AllTests = New Collection AllTests.Add New PricingModuleTests 'AllTests.Add ...


7

You have some nested if statements that I think you could un-nest a little bit For Each item In mCollection If IsEmpty(result) Then result = item Else If item < result Then result = item End If End If Next item so like this instead For Each item In mCollection ...


7

In addition to @RubberDuck's excellent input (which I completely agree with), I have the following concerns: Global Connection Take note that the Database connection object cn is only valid for a single query Nope, that's not how ADODB.Connection works. The reality here is that you have a function that consumes a connection that it doesn't own, ...


6

As of now I see one major problem with this... You are not even testing with the optional parameter. All else equal, you are reusing the same variable and make multiple OPERATE statements. "Each of the tests is clearly split into three parts. The first part builds up the test data, the second part operates on that test data, and the third part checks ...


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