Format RGB Long as hex string in VB6

This is how I convert e.g. RGB(255, 0, 0) (where red is stored in the least significant byte of a Long) to a 6-digit hex string FF0000:

Function RgbToHex(l As Long) As String
Dim r As Long
Dim g As Long
Dim b As Long
    r = l And ColorConstants.vbRed
    g = (l And ColorConstants.vbGreen) / (2 ^ 8)
    b = (l And ColorConstants.vbBlue) / (2 ^ 16)
    RgbToHex = Right$("0" & Hex(r), 2) _
             & Right$("0" & Hex(g), 2) _
             & Right$("0" & Hex(b), 2)
End Function

I'm a bit worried about the string manipulations, but as VB6 lacks printf-like formatting, this is the best I could come up with. Can it be improved?

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. It is certainly not what you want for primitive types like Long. (But it probably is what you want for strings, unless passing them out-of-process, e.g. in a COM server.)

-
As others have pointed out, l is a rather poor choice for a parameter name. Roland says one problem with it is that it can be difficult to distinguish from the digit 1 or a capital I. I'd say that's more a reflection on your choice of a font for your editor, but this is still a valid concern. My major issue with it is that it looks like a lame attempt at Systems Hungarian notation. I know this used to be all the rage in VB programming, but that was mostly cargo-cult. Unless you're using variants (and you probably shouldn't be) the language is sufficiently strongly-typed that embedding type information in the name of the variable serves little point. The only type prefixes I allow myself are for names of controls (mostly because it makes finding them in IntelliSense significantly easier; I always remember the type of control much more readily than I remember its name) and "member" prefixes (e.g., m_). Aside from that, the only prefixes you should be using are meaningful semantic ones, in the Apps Hungarian vein.

Therefore, I would rewrite your function's prototype to look like the following:

Function RgbToHex(ByVal color As Long) As String

Otherwise, the primary issue with your code is that it is slow!

Why? Well, there are a couple of reasons:

-
String operations are extremely slow in VB since they all require multiple BSTR allocations and deallocations, not to mention the fact that the calls to the string-manipulation functions themselves cannot be inlined.

Now, I notice that you did correctly force the use of the string-based Right function by using the $ type suffix, which is a standard trick to ensure as optimal code as possible. This is good. You didn't do the same thing for Hex. Personally, I have no idea whether it is actually necessary or not—is there a variant-based version of Hex? Maybe, I don't know. And I don't want to have to worry about it when I write or review code, so I always use the $ type suffix when I am working with strings. I suggest that you do the same. (Mat's Mug says Hex does have a variant version, so looks like this is sage advice after all!)

But the major problem here is that you're calling six different string-manipulation functions, plus concatenating those three intermediate strings. Slooooow! To optimize the code, it will be critical that we find a way to keep these calls to a minimum.

-
You are using floating-point operations in your byte-extraction code.

In your defense, constant expressions like 2 ^ 8 should be computed by the compiler at compile-time, so that your code never actually has to do an exponentiation operation. I would have made exactly that same assumption. Unfortunately, the VB 6 compiler (even with all optimizations enabled) is not that smart. These two exponentiation operations are done at run-time, with a call to the vbaPower function exported by the run-time library. Personally, I'd say this is a bug in the compiler, but it's not going to get fixed now, and you have no choice but to work around it.

The other bad news is, once you've got a floating-point value, the entire arithmetic operation has to be done in floating-point mode. So for each of your bit-extraction operations, the compiler is going to emit code that converts an integer value into a floating-point value, loads floating-point constants, performs an exponentiation operation, fixes up the result, and then does a division.

While it is true that floating-point operations are no longer as slow on modern machines as they used to be, generally obsoleting the classic optimization advice to avoid floating-point operations whenever possible, VB 6 is not sufficiently modern for that advice to hold. :-) There is no way to get the VB 6 compiler to emit SSE instructions, so you're stuck with the legacy x87 FPU, and that means that you're stuck with a bottleneck intrinsic in its design, arising from the inability to move values directly from the floating-point stack into integer registers and vice versa. The compiler emits code to handle all of this for you seamlessly, but it's still slow. So, at least in the context of VB 6, it's still valid optimization wisdom not to inter-mix floating-point and integer operations.

In this case, though, you don't need floating-point operations at all! You could have done everything in integer mode, just by explicitly using an integer division:

```
r = (l And ColorConstants.vbRed)
g = (l And Col

StackExchange Code Review Q#149900, answer score: 14