Error Handlers / Rolling Back procedures

[djburnheim]

I'm just trying to clean up some code and am trying to workout how to handle erros in a For/Next function. This is what I have at the moment..

***code***

Private Sub cmdStats_Click()

Dim FileName, Available, Logged As String

Available = "J:\Tsdshared\CallCentre\Weekly stats\Tech Team\Available*.csv"
Logged = "J:\Tsdshared\CallCentre\Weekly stats\Tech Team\Logged*.csv"
FileName = Available

For f = 1 To 2

On Error GoTo ErrorHandler

Workbooks.OpenText FileName:=FileName, DataType:=xlDelimited, Comma:=True
Sheets(1).Select
Sheets(1).Move After:=Workbooks("TECH Not Ready Times(v2).xls&quot.Sheets(SheetNum)
FileName = Logged
SheetNum = SheetNum + 1
Sheets(1).Select

Next f

Kill Available
Kill Logged

frmWeeklyStats.Show

ErrorHandler:
MsgBox "There are no ACD Stats!", vbCritical + vbOKOnly, "Can Not Open ACD Stats"

End Sub

***End Code***

If the error occurs on the first step then there isn't a problem but if it's on the second step I want to revert the workbook back to the way it was before the start of the procedure..hope this makes sense.

Dave

 

[VicM]

djburnheim:

Where are you using this code? Is it in an Excel module or are you trying to do this from an Access database? Is the code posted above an abridged version of the actual code? If it's not, then I see several inconsistencies.

First - In your statement:

Sheets(1).Move After:=Workbooks("TECH Not Ready Times(v2).xls&quot.Sheets(SheetNum)

is TECH Not Ready Times(v2).xls another existing workbook, other than the one this code resides in? Or is it a new workbook you are trying to create?

Although I haven't tried your construction, I believe it can't be done this way. If it's an existing workbook, I think you would first need to establish an instance of the other workbook in code before you can put the sheet into it. You would copy the sheet into the other workbook and then use the Move method in the other workbook to move the sheet where you want.

If it's a new workbook, you would first have to create it in code, and you would have to either remove the empty worksheets that are created in a new workbook, or copy the contents of the sheet to 'Sheet1' in the new workbook and change the name of that sheet. The same would apply to the second sheet.

Second - Also in the same statement, where does SheetNum get initialized? You don't show a Dim statement, and there is no assignment statement for it. (This may be moot given the first problem above.)

Third - You have no GoTo statement immediately above the ErrorHandler: statement. Even if this code ran without an error, the messagebox would still appear to the user.

I think we need to have a little more detailed information as to the mechanisms you are trying to employ. If you could be a little more specific, then we might be able to make some suggestions.

HTH,

Vic

 

[Nikita6003]

Hi Dave,

this is probably an overly simplistic view (its getting late over here ;-)) but it seems that you're making a few changes to some workbooks & you want to be able to rollback on error.

Would it be an idea to put some code that closes these workbooks without saving changes do the trick?

Something like

Workbooks("TECH Not Ready Times(v2).xls").Close xlDoNotSaveChanges

but then for all open workbooks where you want to rollback changes.

If not all changes must be rolled back, make sure you save the workbook where you want to keep some of the changes before you move into the routine you describe above.

Hope this is what you meant

Cheers
Nikki

 

[djburnheim]

I was a bit worried my post wouldn't make sense.

The code is run from within the exsisting workbook "TECH Not Ready Times(v2).xls". The code actually works fine although I haven't copied into the post properly (I do have Sheetnum = 1 in it) and now I'm trying to put in error handlers which, as you can tell (and I've found out) aren't working very well..the code below works but I want to put in some error handlers that do the following.

1 - If Available isn't found the procedure stops
2 - If Logged isn't found the procedure stops and the "available" sheet that was added is removed
3 - If both files are found and the sheets are copied to TECH Not Ready Times(v2).xls the source files are deleted

***code***

Private Sub cmdStats_Click()

Dim FileName, Available, Logged As String

Available = "J:\Tsdshared\CallCentre\Weekly stats\Tech Team\Available*.csv"
Logged = "J:\Tsdshared\CallCentre\Weekly stats\Tech Team\Logged*.csv"
FileName = Available
SheetNum = 1

For f = 1 To 2

Workbooks.OpenText FileName:=FileName, DataType:=xlDelimited, Comma:=True
Sheets(1).Select
Sheets(1).Move After:=Workbooks("TECH Not Ready Times(v2).xls&quot.Sheets(SheetNum)
FileName = Logged
SheetNum = SheetNum + 1
Sheets(1).Select

Next f

Kill Available
Kill Logged

End Sub

***End Code***

Hope this makes what I'm try to do clearer.

 

[Nikita6003]

Why not test FIRST to see if the files you need exist? You can use the Dir("FullpathAnFilename&quot for that. If it returns "" the file you specified doesn't exist

So add to your code:

If Dir(Logged) = "" Or Dir(Available) = "" Then
  Exit Sub
End If

However, if you want a full-blown error handler this might be more useful:

Dim l_wksSheetThatMoved As WorkSheet

On Error Goto Errorhandler


'... code

'This sets an object to the sheet that you're adding to the Tech not ready workbook
'This way xl'll remmeber which sheet was added & you can remove the sheet in your errorhandler
Set l_wksSheetThatMoved = ThisWorkbook.Sheets(1)
l_wksSheetThatMoved.Move After:=ThisWorkbook.Sheets(3)
l_wksSheetThatMoved.Delete

Exit Sub
ErrorHandler:
  if Dir(Logged) = "" then l_wksSheetThatMoved.Delete

  end if

Hope this is more what you wanted ...

Cheers
Nikki

 

[VicM]

djburnheim:

Test for file existence before you run thru your code. You can then branch accordingly. Because both files need to exist for your code to execute, add code like the following to your Sub:

Dim FileName, Available, Logged As String, fs as Object

Available = "J:\Tsdshared\CallCentre\Weekly stats\Tech Team\Available*.csv"
Logged = "J:\Tsdshared\CallCentre\Weekly stats\Tech Team\Logged*.csv"

If fs.FileExists(Logged) And fs.FileExits(Available) Then
code to do what you want
Else
Exit Sub
End If

HTH,

Vic

 

[VicM]

dJ

Missed one statement that is necessary. After the Dim statement, add:

Set fs = CreateObject("Scripting.FileSystemObject&quot

I got a bit ahead of my self.

Vic

 

[djburnheim]

Thanks everybody for their help...I've used Nikki's suggestion.

Being quite new to vba and trying to teach myself from the built in help and through the helpful people in this forum, I've realised that there always seems to be more than one way to do things...how do you choose the best way? I have created a workbook with a fair bit of code and it is all working but I'm sure I've gone about things the long/wrong/stupid way, can anybody suggest anywhere I can find pointers for "best practice" in vba?

Thanks again
Dave

 

[Nikita6003]

Glad we could help ;-)

I guess it's a question of putting it all into practice. Can't speak for others in this forum, but believe me - I've leart most from doing things the wrong way

best resource are sites like this, and, now I'm a little bit more familiar with the Office Object model, I tend to use the object browser a ot (In VBA press [F2]) This lists all objects & their associated methods, event & properties. With the NAME of what I want, I can then either find what I ned in VBA Help files (even tho' the info there is quite often just plain wrong), or google the name/g to Tek-Tips or Expert-Exchange (another good forum) etc.

Cheers
Nikki

 

[Nikita6003]

Oh, and watch this thread thread707-474222 for more useful links; I get the idea the list there will grow & grow ...

Nikki

 

[rmikesmith]

VicM,

To clarify things for you or others: A statement such as

Worksheets("Sheet1").Move After:=Workbooks("AnotherWorkbook.xls").Worksheets("SomeSheet")

is perfectly legal, as long as the second workbook is open. If you don't specify the After parameter (or its companion, Before) then the worksheet is moved to a new workbook.


HTH
Mike

 

[VicM]

rmikesmith:

You are correct Mike, if the workbook is already open. I guess I was only considering the scenario where it existed and was not open. Under that condition, one would have to establish an instance of the workbook in code in order to carry out the copy/move.

Of course, one would also need to test whether that workbook existed.

Since Dave does use the After argument, and if the workbook is closed, I assumed that would throw an error.

Vic

 

[rmikesmith]

Vic,

Yes, checking for the existence of the workbook and whether it is open is certainly the way to go. Using the After or Before parameter with the workbook closed will throw a 'Subscript out of range' error.


Regards,
Mike

 

[tmktech]

Interesting Nikita.

I've been using the filesystemobject for existence of files, but find the DIR approach intriguing. Do you find it just as robust? Any pro / cons to your approach?

Thanks in advance!

TMKTECH

 

[Nikita6003]

It's pretty robust - I use it to fill a an array for a quick test to see if my source files have all been delivered (if one's missing I can't run the import routine, you see) & it's never gone wrong for me yet.

Pros: it's 1 line of code against setting the fso, then using it.
Cons: haven't found any really, apart from the fact that fso can do everything file-related.

I think I probably use it for its simplicity. Makes for easy code reading and as each member of the team I work with has to be the backup for every other member, that's a big consideration for us.

Cheers
Nikki