>

LOCAL lnI, laDbfs[1]
>
>CREATE CURSOR badTables ;
>( ;
>    cName     c(40), ;
>    cMessage  c(200) ;
>)
>
>SELECT 0
>ADIR(laDbfs, "*.DBF")
>
>ON ERROR INSERT INTO badTables VALUES(laDbfs[lnI,1], MESSAGE())
>FOR lnI = 1 TO ALEN(laDbfs,1)
>    USE (laDbfs[lnI,1])
>NEXT
>ON ERROR
>
>SELECT badTables
>GOTO TOP
>BROWSE NOWAIT
>CANCEL
>

Yay Rick!!!

We're finally getting somewhere! You did not use any PUBLIC variables. EVERY routine should be self-contained. If you use publics, then a timer could alter the public variables in the routine and break it. Do you have any idea how hard that would be to debug? Locals are much better. By instinctively or habitually avoiding public and opting for local first, you simply must make better code. I know you don't like the look of mdots, but you are leaving your code open to an accidental field/memvar conflict - it is not encapsulated. In this case, since it is opening every table, the chances of a conflict are higher.

I agree with Tore, reset the on error - if you use it. You are way better off using TRY...CATCH though. Structured error handling is the better newer way. In this example, a global on error handler would not be the thing to call. That's why you'd have to disable/bypass it as you did. That requires you use macro-substitution to re-enable it. That is another thing I avoid habitually because it is slower than other options. TRY...CATCH can test the use command without any interference from the global error handler. It's much more specifically targeted.

I like that you did not need to close the table after each USE. I should have thought of that. We can all make mistakes - as long as we are trying to use the best practices. Although there may be something to closing the table immediately if it's good enough to open, so as to not impact any other modules. I added SHARED and NOUPDATE to reduce risk to an existing system. One thing you missed was to close the last file opened. :)

The list of bad tables in a cursor or table is good but I'm quite tempted to have the cursor created before calling this routine. The routine should clean up completely and not leave anything hanging open. If a calling routine created the cursor and then called this routine, passing the name of the cursor, that is tighter. The called routine could put things back as it found them.

I also added a parameter to specify the folder to examine.

So the calling routine

IF NOT USED("c_BadFiles")
	CREATE CURSOR c_BadFiles (cFileName c(200), cMessage c(200))
ENDIF
=GetBadDbfs("","c_BadFiles")

and here's the modified GetBadDbfs

LPARAMETERS m.tcFolder, m.tcBadFiles
* Loop through specified folder to find "bad" dbf
* files by trying to open each. Log failures.

LOCAL lcFolder, lcTable, loError, lcAlias

IF EMPTY(m.tcFolder)
	lcFolder = FULLPATH(CURDIR())
ELSE
	lcFolder = ADDBS(m.tcFolder)
ENDIF

lcAlias = ALIAS()

IF EMPTY(m.tcBadFiles) OR NOT USED(m.tcBadFiles)
	RETURN .F.
ENDIF

SELECT 0
lcTable = SYS(2000,m.lcFolder + "*.DBF")
DO WHILE NOT m.lcTable == ""
	TRY
		USE (m.lcFolder + m.lcTable) SHARED NOUPDATE
	CATCH TO loError
		INSERT INTO (m.tcBadFiles) (cFileName, cMessage) VALUES (FULLPATH(m.lcFolder) + m.lcTable,m.loError.Message)
	ENDTRY
	lcTable = SYS(2000,m.lcFolder + "*.DBF",1)
ENDDO

USE IN (SELECT())

SELECT (IIF(m.lcAlias=="",0,m.lcAlias))
RETURN