Excel VBA Guru needed....

[WhiteD3]

Folks,

I need a VBA guru to help me with a problem please that has me stumped.

At work we use a lot of spreadsheets that are protected and use VBA for basic operations like add or copy a row. We've been using iterations of these spreadsheets for years and the VBA code is basic, prooven and long lived.

Recently one of the guy has started getting a runtime error -2147417848 "The object invoked has disconnected from its client". The error ocurs randomly.

No one else in the office has had the problem and so I dismissed the issue as user related. Until yesterday when it happened to me countless times. Now I can't replicate it although its still happening to him, on two laptops!

I think the issue is related to this INFO: Error or Unexpected Behavior with Office Automation When You Use Early Binding in Visual Basic (http://support.microsoft.com/kb/319832). I modified the code with Option Explicit and declarations but the problem persists.

What I'd like to do is to email the spreadsheet to a guru for a look at the code.

Help would be greatly appreciated :angel:

[TeamFA]

I would rate myself as an average VBA user, but I play with it in Access, not Excel.

Happy to waste some of my time looking at it, if you want to email.

If anybody else has more l33t s|<1llz, jump right in.

forsythem1972gmail.com

[Ferret]

I do some excel based VBA work commercially.

If you wish I'll have a look - but no promises. First the error has to reproducible and it depends on other things like how well the code is documented, how much time needs to be put into etc.

PM me with an email address if you wish to make contact.

Have you upgraded recently to a different version of Office. I have found office 2010 to break previously working excel vba code I maintain.

[WhiteD3]

"Reproducable"? Wouldn't that be nice!

We all run Win 7, 64 bit laptops, Office 2010.

PM sent.




I do some excel based VBA work commercially.

If you wish I'll have a look - but no promises. First the error has to reproducible and it depends on other things like how well the code is documented, how much time needs to be put into etc.

PM me with an email address if you wish to make contact.

Have you upgraded recently to a different version of Office. I have found office 2012 to break previously working excel vba code I maintain.

[dmdigital]

Are you using Office 2010 32-bit or 64-bit version? 64-bit can do all sorts of funny things.

Also the amount of RAM in the PC's can impact on Excel iterations as Excel still leaks memory as much as it use ever has.

As for the VBA a few things to check...
Does the spreadsheet have any potential particularly via the code to create circular references?
What reference libraries is the VBA using? Is there newer ones being used or that can replace older ones? This could also relate to the link you posted.

I would definitely be declaring everything, always use Option Explicit and clear objects by setting them to nothing when no longer used.

I've encountered this error in every version of office since vBA came along I'd hazard a guess. Often you can't use debug successfully on this type of error but you can use Debug.Print statements to narrow down where in the code it is occurring.

[WhiteD3]

Thanks Derek.

The OS is definitely W7 64 bit but I'm not sure about Excel 2010. His laptop will be a basic Dell with maybe 4 GB RAM where as mine is a hi spec Asus with 12 GB RAM.

Re the code; no circular references and I'll have to have a look at the libraries. There's been nothing installed or added to these laptops beyond the standard install.

One thing I'm not doing is clearing the objects after use.

Cheers,

Mark

[dmdigital]

If you need someone to pass an eye over any code snippets let know. I have done too many millions of lines of code including a lot in VBA in MSOffice. I don't run any MSOffice products at home though so can't debug anything for you.

[WhiteD3]

Gents,

Thanks for your help above. I've modified the code to qualify objects, declaring variables and setting them to 0 after use. As a result the crashes have gone from frequent to sometimes.

Below is the module and form code for copying rows. This is the most used code in the spreadsheet and results in the most errors. Always the same "disconnect from client" error.

I'd really appreciate any comments on what I've missed in my clean up.

Cheers,

Mark

Modules:

Option Explicit
Public Number_of_Rows, Copy_Start_Row, Copy_End_Row, Copy_Insert_Row, i As Integer

Sub Row_Add()
'Adds rows in multiples of 1, 5, 10 or 20 depending on what the user selects. Rows can be added anywhere within a section.
'Dim Number_of_Rows As Integer - Is declared as a Public variable at the top of this module as "Number_of_rows" is passed between this sub and the Add_Rows form code.

Number_of_Rows = 0 'Value assigned in the fmAdd_Rows form.
'Check that the current active cell is a valid selection
If ActiveCell.Row <= Range("PT_Start").Row + 1 Or ActiveCell.Row >= Range("PT_Total").Row - 2 Or Left(Cells(ActiveCell.Row, 1).Value, 8) = "Subtotal" Or Cells(ActiveCell.Row, 2).Value = "DO" Then
MsgBox "You must select a cell within a section"
GoTo Skip_Add
End If
'The current selection is valid so.......
fmAdd_Rows.Show 'Load the Add_Rows form to get the number of rows to be added.

ThisWorkbook.Worksheets("Points Takeoff").Unprotect "sysio"
'Copy and paste the template row
If Number_of_Rows = 0 Then 'The user has selected Cancel instead of 1, 5, 10 or 20 rows.
GoTo Skip_Add
ElseIf Number_of_Rows = 1 Then
ThisWorkbook.Worksheets("Points Takeoff").Range(Cells(Range("PT_Add_Row_1").Row + 1, 1), Cells(Range("PT_Add_Row_1").Row + 1, 1)).EntireRow.Copy
Selection.Insert Shift:=xlDown
ElseIf Number_of_Rows = 5 Then
ThisWorkbook.Worksheets("Points Takeoff").Range(Cells(Range("PT_Add_Row_5").Row + 1, 1), Cells(Range("PT_Add_Row_5").Row + 5, 1)).EntireRow.Copy
Selection.Insert Shift:=xlDown
ElseIf Number_of_Rows = 10 Then
ThisWorkbook.Worksheets("Points Takeoff").Range(Cells(Range("PT_Add_Row_10").Row + 1, 1), Cells(Range("PT_Add_Row_10").Row + 10, 1)).EntireRow.Copy
Selection.Insert Shift:=xlDown
ElseIf Number_of_Rows = 20 Then
ThisWorkbook.Worksheets("Points Takeoff").Range(Cells(Range("PT_Add_Row_20").Row + 1, 1), Cells(Range("PT_Add_Row_20").Row + 20, 1)).EntireRow.Copy
Selection.Insert Shift:=xlDown
End If

ThisWorkbook.Worksheets("Points Takeoff").Protect "sysio"
Skip_Add:
Number_of_Rows = 0
End Sub


Forms:

Private Sub UserForm_Initialize()
'This sub, called from the Points_Takeoff module, Copy_Rows sub, is called Userform_Initialize as the code resides in this form, therefore do not change the sub name.
'Set defualt values of the textboxes to zero as their default blank value is text, which is invalid when read into the 3 integer variables below.
tbStart_Row.Value = 0
tbEnd_Row.Value = 0
tbInsert_Row.Value = 0

tbStart_Row = Selection.Row
tbEnd_Row = Selection.Rows.Count + Selection.Row - 1

End Sub

Private Sub cbSelect_Range_Click()
tbStart_Row = Selection.Row
tbEnd_Row = Selection.Rows.Count + Selection.Row - 1
End Sub

Private Sub cbInsert_Row_Click()
tbInsert_Row = Selection.Row
End Sub

Private Sub cbOK_Click()
'Checks the validity of the selections, then copies and pastes the selections.
Copy_Start_Row = tbStart_Row.Value
Copy_End_Row = tbEnd_Row.Value
Copy_Insert_Row = tbInsert_Row.Value

'Check that the current copy and paste selections are valid.
If Copy_Start_Row < 11 Or Copy_End_Row < 11 Or Copy_Insert_Row < 11 Then 'Checks that all 3 rows selected not blank and are at least within the first section.
MsgBox "You must entered a row number in the First, Last and Insert boxes"
GoTo Skip_Copy
End If
'Loop through the copy range checking selection is valid.
For i = Copy_Start_Row To Copy_End_Row
If i <= Range("PT_Start").Row + 1 Or i >= Range("PT_Total").Row - 2 Or Left(Cells(i, 1).Value, 8) = "Subtotal" Or Cells(i, 2).Value = "DO" Then
MsgBox "You must select copy cells within a section"
GoTo Skip_Copy
End If
Next i
'Check the insert cell selection is valid.
If Copy_Insert_Row <= Range("PT_Start").Row + 1 Or Copy_Insert_Row >= Range("PT_Total").Row - 2 Or Left(Cells(Copy_Insert_Row, 1).Value, 8) = "Subtotal" Or Cells(Copy_Insert_Row, 2).Value = "DO" Then
MsgBox "You must select an insert cell within a section"
GoTo Skip_Copy
End If

'The current selection is valid so call the Copy sub below.

Copy_Rows

Skip_Copy:

End Sub

Private Sub cbCancel_Click()
'This is the code for the Cancel button, which closes the form without saving any changes.
Unload fmCopy_Rows 'Use of Unload is to remove the form from memory so that each time the user runs the form, the list is fresh.
End Sub

Private Sub cbHelp_Click()
'fmCopy_Rows.Hide
Unload fmCopy_Rows
fmHelp.Show
End Sub


Sub Copy_Rows()
'Called from cbOK_Click sub above.
'Copies and inserts rows.
ThisWorkbook.Worksheets("Points Takeoff").Unprotect "sysio"
ThisWorkbook.Worksheets("Points Takeoff").Range(Cells(Copy_Start_Row, 1), Cells(Copy_End_Row, 1)).EntireRow.Copy
ThisWorkbook.Worksheets("Points Takeoff").Cells(Copy_Insert_Row, 1).EntireRow.Select
Selection.Insert Shift:=xlDown
ThisWorkbook.Worksheets("Points Takeoff").Protect "sysio"

Copy_Start_Row = 0
Copy_End_Row = 0
Copy_Insert_Row = 0
End Sub

[FeatherWeightDriver]

What line (or lines) of code do you get errors on?

PS: I would suggest setting variables to nothing rather than 0 after you are finished with them.

[WhiteD3]

What line (or lines) of code do you get errors on?

PS: I would suggest setting variables to nothing rather than 0 after you are finished with them.

The error (always the same one) occurs randomly when running any code is the spreadsheet. "Nothing" noted.

Thanks.

[FeatherWeightDriver]

Randomly eh? My favourite kind of error :mad:

[Ferret]

Mark
Your error arises because something is going wrong with memory management in the background. This is just my feeling, maybe I'm wrong.

As said previously by PM your code is basically unsafe because there is room for ambiguity to arise when multiple workbooks are opened within the same Excel thread. And you have said your uses typically have many workbooks open, including "multiple copies of this workbook" :eek:. I assume they must be named slightly differently.

You allow users to open a 'modeless' form. This means they can navigate away from the sheet it was originally opened from to any other sheet in your current workbook, or even any sheet in any other workbook opened within the same Excel thread and yet still interact with your form.

You make the assumption throughout your code that the user is not doing this by referring to objects which are basically unqualified. When a reference is said to be unqualified it means there is scope for ambiguity to arise.

For instance:

ThisWorkbook.Worksheets("Points Takeoff").Cells(Copy_Insert_Row, 1).EntireRow.Select

Which workbook are you referring to here? The workbook from which your user originally opened the modeless form or the workbook that is currently active. Your application will crash if the user is not currently using the 'right' workbook or it may even continue to function if the currently active workbook just happened to have a sheet named "Points Takeoff" within it. The point is you don't really know what will happen when multiple workbooks are open, particularly if there is a chance the workbooks are 'clones' of each other.

You need 'Workbooks("BlahBlahBlah").Worksheets("Points Takeoff").Cells(Copy_Insert_Row, 1).EntireRow.Select

No ambiguity possible here.

Another example

Private Sub cbInsert_Row_Click()
tbInsert_Row = Selection.Row
End Sub

1) What is 'tbInset_Row', is it an integer, a string, an object?. You know it is an a text box object on one of your forms but the compiler does not unless it works it out by first scratching its head and then trying to figure out what you mean. Do yourself a favour and give it a hint - ie

Private Sub cbInsert_Row_Click()
Me.tbInsert_Row.Value = Selection.Row
End Sub

2) Exactly what 'Selection' are you referring to here. It is of course the selection on the currently active sheet but you, I think, are assuming it is a selection on the sheet from which the user originally opened the form. This may not be the case because you allow users to navigate away to any sheet.

Some other points
You have about 150 'named range objects' buried in your application. About 100 of these point no where, ie they reference cell ranges which don't exist. They need to be removed.

'Option Explict' should be used everywhere. It should never be optional.

Don't use public variables. Pass parameters directly to subroutines. There is far less scope for confusion.

Just some things to keep you thinking.

[WhiteD3]

Thanks Ferret. That should keep me awake for a few nights! :o

What I don't get (and not making excuses for my crappy programming) is why this has started to happen. We (the business) have been running this spreadsheet for 7 years with zero problems. All of a sudden I've got one user with frequent crashes, 2 with infrequent crashes and the rest of us (say 5 users) with no crashes.

It's enough to send me mad(er).

:wallbash::wallbash::wallbash::wallbash:

Oh....I must confess that I was being cute (and lazy) using "ThisWorkbook".

[Ferret]

What I don't get (and not making excuses for my crappy programming) is why this has started to happen.

I don't know exactly either. You may have been using the same code for years but you have not been using the same system configuration for years.

There are memory management processes running in the background which you generally can't see or interact with. They are responsible for keeping track of which instance of objects are in memory and which instance of objects can be freed from memory (ie destroyed) when it is determined they are no longer needed.

Just when an object is no longer needed (ie moves out of scope or becomes unreachable) can be tricky to determine.

This process is referred to as 'garbage collection'. In the case of VB / VBA and MSOffice I think it is somewhat simpler process referred to as 'reference counting' (https://en.wikipedia.org/wiki/Reference_counting#COM).


Bugs caused by incorrect reference counting in COM systems are notoriously hard to resolve, especially because the error may occur in an opaque, third-party component.

This is why it is good practice (sometimes required practice) to fully qualify references and explicitly define datatypes and also why your problem appears to be random among different users. Again I stress - "I think".

As well a making sure all reference are explicitly qualified I would also make sure users can not navigate away from the sheet that raised the form until they are done with using the form. I would also make it impossible to push the button on the spreadsheet which raises the form while the form is already displayed.

In other words, tie down your application so it can't be used in ways you don't and never intended it to be used.

[WhiteD3]

Peter et al,

Just some feed back......I've got all the objects qualified now, forms Modal, variables declared and cleared as required. Of course there hasn't been a crash for a week :BigCry: with the guys still using the old file that had only some of the changes above completed.

The main protagonist actually called yesterday to say how well things were going................which was great until I pointed out he's using the same file that's been causing all the grief lately.

This, like many things, is a mystery to me.

Thanks for your help and advice............although I'm sure I'll be back :eek:

[Ferret]

Don't ya love it :twisted: