Code Critic Revisited

Last week I posted some code for you to criticize. There were some great comments and now I have some questions.

rzf said:
The code looks good and is easy to read, but if you want it to be foolproof, I can work around it in a few ways:
1. Drag and drop moving of a cell
2. Paste values
3. Paste formulas
4. Just hit escape after getting the error message

1. Yeah, that’s a good one. Dragging and dropping definitely breaks the code. Until we get a BeforeDrop event, I can’t think of any way around this one.
2. Pasting values and formulas did not remove the validation for me. In fact, what I thought of doing instead of recreating the validation was forcing a Paste Values. That would mean that I would have to redefine all the ways the user could paste to Paste Values, then put them back. It seemed like more work than just recreating the validation, but I don’t know.
4. Truth. I had to hit escape twice to get it to screw up. It didn’t delete the validation, but invalid data still ended up in the cell.

Jan said:
The program does not behave the way I would like it to behave.

I agree. My test of after-paste validation should look like Excel’s with the Critical mark, same text and Cancel and Retry buttons.

sMsg = “The value you entered is not valid.” & _
&nbsp&nbsp&nbsp&nbspvbCrLf & vbCrLf & _
&nbsp&nbsp&nbsp&nbsp”A user has restriced values that can be entered into this cell.”
&nbsp
lResponse = MsgBox(sMsg, vbRetryCancel + vbCritical)
If lResponse = vbCancel Then
&nbsp&nbsp&nbsp&nbspApplication.EnableEvents = False
&nbsp&nbsp&nbsp&nbspTarget.Value = mvOldValue
&nbsp&nbsp&nbsp&nbspApplication.EnableEvents = True
Else
&nbsp&nbsp&nbsp&nbspSendKeys “{F2}” ‘I hate sendkeys, but what to do
End If

Tony said:
Error handling only gets switched off if there is no error

Good point on the error handling. Like this?

On Error Resume Next
&nbsp&nbsp&nbsp&nbspsTemp = Target.Validation.ErrorMessage
&nbsp&nbsp&nbsp&nbsplErrNo = Err.Number
On Error Resume Next
&nbsp
If lErrNo = 1004 Then ‘it does not
&nbsp
&nbsp&nbsp&nbsp&nbspErr.Clear

That was Mike’s suggestion. I like the idea of indenting, but when do you outdent? If I set up a variable to hold err.number, I can indent like the above. If I don’t, then I’ll have two On Error Resume 0’s which will clear my error number. I don’t like having to set up a new variable, but I think it’s worth it for the readability of indenting the error block.

You could move the ‘With Target.Valuation’ a few lines higher, eg to include the .Add statement

That’s true. I do it the way I did because I like it better that way. I think it’s easier to read, but it’s definitely a personal preference. Or am I the only who does it like this?

…most people use the prefix m for public variables

I’m the first one to complain about bad variable names and the first one to use bad variable names. You’d think I could at least make the effort when I’m inviting criticism.

Dim mxAlertStyle As XlDVAlertStyle
Dim msErrMsg As String

m for module level, s for String. But what do you use for xlDVAlertStyle? It’s probably a Long internally, so maybe I should use that. I chose x, but I don’t know if I like it. Mike suggested this too.

Mike said:
Those two guard clauses could then perhaps move into another function…

Oh, that’s good. They should be in another function. Readable, self-documenting. That’s a peach of a suggestion.

John said:
And it’s a lot simpler.

Always taking the easy way out. My goal was to allow the user to paste valid data, but with rzf’s comments, that may just be a pipe dream. Your code is probably better in that it just prevents the problem rather than trying to work around it.

If you were going to make an add-in, how you would get ValidationRange set up so that it works on any workbook? I’m thinking a class module with Application level events. When you add a workbook to the Workbooks collection, it would store a range that was all the cells with validation. Then you’d have to figure out a way to update that when validation was added. Ugh. Also, drag and drop screws up ValidationRange.

This may be one of those bugs that it’s just not worth trying to workaround. But I learned some good stuff from your comments, so thanks.

Posted in Uncategorized

One thought on “Code Critic Revisited

  1. I’m glad to have stumbled across your blog and in particular the Code Critic aspect of it. This can prove to be a very valuable resource in dealing with the risks inherent in the fragile and delicate spreadsheet environment.

    Each new spreadsheet is in essence a new custom program made by someone that is (circa 95% of the time) not trained as a programmer.

    The The Fragile Last Mile of BI: Spreadsheet Risk & Fraud Analysis blog is dedicted to raising awareness of this global promblem.

    The link is http://blogs.ittoolbox.com/bi/spreadsheet/archives/000613.asp

    and also look at

    http://blogs.ittoolbox.com/bi/spreadsheet/archives/000706.asp


Posting code? Use <pre> tags for VBA and <code> tags for inline.

Leave a Reply

Your email address will not be published.