Tek-Tips is the largest IT community on the Internet today!

Members share and learn making Tek-Tips Forums the best source of peer-reviewed technical information on the Internet!

  • Congratulations Chris Miller on being selected by the Tek-Tips community for having the most helpful posts in the forums last week. Way to Go!

Speed up excel import automation? 3

Status
Not open for further replies.

EzLogic

Programmer
Aug 21, 2001
1,230
US
I have a small form that takes an excel file, opens the file, and traverse thru it and imports it.
VFP 2 / SP2
windows 2012 server 64 bit (my dev machine)
Office 2007 Excel.

It is going really slow. Is there a better/faster way to do this?

ActDate_Cell, Mastername_Cell, etc.. is the cell of the excel coming from a template file.
example: A, B, C, AA, etc..

the looping thru the workbook is slow..

Code:
TRY 
	llReturn = .f.
	loExcel = CREATEOBJECT("Excel.Application")
	loExcel.Workbooks.open(lcFile)
	oSheet = loExcel.ActiveSheet
	loExcel.visible = .t.
	llOK = .T.
	X = 1
*	SET STEP ON
	DO WHILE llOK

			y = TRANSFORM(x)
			m.ActDate	 	= NVL(loExcel.Range(ActDate_Cell+y).value,{})
			m.MDN			= NVL(UPPER(ALLTRIM(TRANSFORM(loExcel.Range(mdn_Cell+y).VALUE))),'')
			m.Master		= NVL(UPPER(ALLTRIM(TRANSFORM(loExcel.Range(master_Cell+y).VALUE))),'')
			m.MasterName	        = NVL(UPPER(ALLTRIM(TRANSFORM(loExcel.Range(mastername_Cell+y).VALUE))),'')
			m.sub			= NVL(UPPER(ALLTRIM(TRANSFORM(loExcel.Range(sub_Cell+y).value))),'')
			m.subname		= NVL(UPPER(ALLTRIM(TRANSFORM(loExcel.Range(subname_Cell+y).value))),'')
			m.Serial		= NVL(UPPER(ALLTRIM(TRANSFORM(loExcel.Range(Serial_Cell+y).VALUE))),'')			
			m.MRC			= NVL(VAL(TRANSFORM(loExcel.Range(mrc_Cell+y).value)),0)
			m.Rate			= NVL(VAL(TRANSFORM(loExcel.Range(Rate_Cell+y).value)),0)
			m.Amount		= NVL(VAL(TRANSFORM(loExcel.Range(amount_Cell+y).value)),0)

			IF TYPE("m.ActDate") <> 'D'
				m.ActDate = CTOD(SUBSTR(TRANSFORM(m.ActDate),1,8))
			ENDIF 

		
		IF m.serial = '.NULL.'
			m.serial = ''
		ENDIF  

		
		IF m.serial = '.NULL.'
			m.serial = ''
		ENDIF 

		IF NOT EMPTY(m.serial) AND !ISNULL(m.serial) AND m.serial <> '.NULL.'
				IF m.serial <> 'SIM'
					SELECT (lcCurCom)
					m.serial = SUBSTR(m.serial,1,20)
					APPEND BLANK 
					GATHER MEMVAR 
					thisform.grdCom.Refresh()
				ENDIF 
				X = X + 1
		ELSE
				EXIT
		ENDIF
	ENDDO
	loExcel.quit

CATCH TO loexp
	MESSAGEBOX("Error importing: " + loExp.Message + CHR(13) + ;
			   "Line No: " + TRANSFORM(loExp.LineNo),16,thisform.Caption)
	llreturn = .t. 
	IF TYPE('loExcel') = 'O'
		loExcel.Quit
	ENDIF 
	EXIT 
ENDTRY




Ez Logic


Ez Logic
Michigan
 
the excel file format changes. (columns added/removed) never the same, every other months its different.

but, the columns that I need are there, but, in different orders, etc..

hence I have a template, which tells the system, where each column is.

Short answer, No. I wish.

Ez Logic
Michigan
 
the excel file format changes. (columns added/removed) never the same, every other months its different.

One thing that I have done for this type of situation is to use Excel Automation to initially open the Excel file and write out just the first Header Row as a CSV file after which close the Automation.

Then have the VFP application 'read' the CSV file to determine the Field name & sequence needs - which it then uses to build a recipient data table (all fields are type: Character of a larger than needed size).

Finally do the APPEND that Dave suggests above.

Once the data is in a VFP data table, your application can do whatever it needs to do with it - change field types, extract your specific field values or whatever.

While there are a number of steps involved, the preliminary ones don't take much time to execute.

And, if you have a LOT of Excel rows, the APPEND will be faster than your current approach.

Good Luck,
JRB-Bldr
 
One thing that jumps out at me is that every reference starts with loExcel. That means that on every call, VFP has to start with your object reference and swim down to the Range collection.

How about this:

Code:
With loExcel
   * other code and stuff...
   .Range(... etc.)
Endwith

You shouldn't write VFP code that makes repeated references to the same object:

Code:
This.something()
This.anotherthing()
etc.

It's doubly important when you're crossing the COM chasm.
 
My first reaction is that it's not surprising this is slow. You have ten calls to your Excel object for each iteration of your main loop. The fact is that calling any COM object involves an overhead, and doing it that many times will multiply that overhead considerably.

I agree with Dave that you should try and do this with an APPEND FROM if at all possible. You say that the column layout changes each time. I have been in that same situation - essentially, you know that some or all of the columns are present, but you don't know which order they are in.

Basically, what you need to do is to do the APPEND FROM first. Then, once you have the data in a VFP cursor, you work out which columns are present, and which cursor fields they correspond to. The point is that you do that entirely in VFP, without using a COM interface.

In your case, it is the Excel range name that is determining the presence/absence and location of the columns. So, once you have done the APPEND FROM, you would use Excel Automation to open the sheet, and then determine which ranges are present and which columns they correspond to. You can then loop through the VFP cursor, extracting the data that corresponds to the columns, and placing this in another cursor, whose field names match the original range names. (This is similar to JRB's suggestion, except that you use range names rather than column headers to determine the mapping of the columns.)

Mike



__________________________________
Mike Lewis (Edinburgh, Scotland)

Visual FoxPro articles, tips and downloads
 
My first reaction is 'VFP 2/ SP2'?

Regards

Griff
Keep [Smile]ing

There are 10 kinds of people in the world, those who understand binary and those who don't.

I'm trying to cut down on the use of shrieks (exclamation marks), I'm told they are not good for you.
 
Thanks guys for the input.

I changed my code initially to use:
Code:
With loExel
   do while lOk 
      ..
   enddo
endwith

Same speed....

Griff, that was a Typo. It's VFP 9 SP2 :)

I am going to modify the code and do the append from and work and map the fields.



Ez Logic
Michigan
 
I figured

B-)

Regards

Griff
Keep [Smile]ing

There are 10 kinds of people in the world, those who understand binary and those who don't.

I'm trying to cut down on the use of shrieks (exclamation marks), I'm told they are not good for you.
 
Great input.. so, i re-wrote my import and now, its takes about 3seconds to 7 seconds, instead of 10min+

Note, that the cells will never be more than 15 columns (no need for AA, AB, AC columns)..

Here is my revised import code:
Code:
LOCAL lcStr, x, loExcel, lcTempfile
lcStr = [CREATE CURSOR curImport ( ]

TRY 

	FOR x = 1 TO 40
		 lcStr = lcStr + ' Col'+TRANSFORM(x) + ' c(100),'
	ENDFOR 
	lcStr = lcStr + ' LastCol c(1) )'
	&lcstr

	lcFile = GETFILE('xlsx;xls')
	IF EMPTY(lcFile)
		EXIT 
	ENDIF 
		
	loExcel = CREATEOBJECT("Excel.Application")
        with loExcel 
	   .DisplayAlerts = .f.
   	   .Workbooks.open(lcFile)
	   oSheet = .ActiveSheet
	   .visible = .t.
	   lcTempfile = SYS(5)+"\ezcellerp\"+SYS(2015)+".csv"
	   .ActiveWorkBook.SaveAs(lcTempFile,6)
           .Workbooks.Close()
        endwith 

	SELECT curImport
	APPEND FROM (lcTempFile) TYPE CSV 
	ERASE (lcTempFile)
	LOCATE 
	IF EOF()
		MESSAGEBOX("File imported did not containt any information.",16,_screen.Caption)
		USE IN SELECT('curImport')
		EXIT 
	ENDIF 


	


	SELECT H2OBoltResidualTemplate
	LOCATE 
		
	lcActDateCol 	= Column2Field(ActDate)	
	lcMDNCol 	= Column2Field(MDN)	
	lcMasterCol 	= Column2Field(Master)	
	lcMasterNameCol = Column2Field(MasterName)	
	lcSubCol 	= Column2Field(Sub)	
	lcSubNameCol 	= Column2Field(SubName)	
	lcSerialCol 	= Column2Field(Serial)	
	lcMRCCol 	= Column2Field(MRC)	
	lcRateCol 	= Column2Field(Rate)	
	lcAmountCol 	= Column2Field(Amount)	

	IF EMPTY(lcActDateCol) OR EMPTY(lcMDNCol) OR EMPTY(lcMasterCol) OR EMPTY(lcMasterNameCol) OR ;
	   EMPTY(lcSubCol) OR EMPTY(lcSubNameCol) OR EMPTY(lcSerialCol) OR EMPTY(lcMRCCol) OR ;
	   EMPTY(lcRateCol) OR EMPTY(lcAmountCol)
	   	
	   	MESSAGEBOX("Unable to import file.  Check Template file and try again.",16,_screen.Caption) 
	   	EXIT 
	ENDIF 
	
	SELECT curImport
	SCAN
		m.ActDate 	= CTOT(UPPER(ALLTRIM(&lcActDateCol)))
		m.MDN		= UPPER(ALLTRIM(&lcMDNCol))
		m.Master	= UPPER(ALLTRIM(&lcMasterCol))
		m.MasterName    = UPPER(ALLTRIM(&lcMasterNameCol))
		m.Sub		= UPPER(ALLTRIM(&lcSubCol))
		m.SubName	= UPPER(ALLTRIM(&lcSubNameCol))
		m.Serial	= UPPER(ALLTRIM(&lcSerialCol))
		m.MRC		= STRTRAN(ALLTRIM(&lcMRCCol),[$],[])
		m.MRC		= STRTRAN(m.MRC,[,],[])
		m.MRC		= VAL(m.MRC)
		m.Rate	        = STRTRAN(ALLTRIM(&lcRateCol),[$],[])
		m.Rate		= STRTRAN(m.Rate,[,],[])
		m.Rate		= VAL(m.Rate)

		m.Amount        = STRTRAN(ALLTRIM(&lcAmountCol),[$],[])
		m.Amount	= STRTRAN(m.Amount,[,],[])
		m.Amount	= VAL(m.Amount)

		INSERT INTO (lcCurCom) FROM MEMVAR 
	ENDSCAN 	

CATCH TO loexp
	MESSAGEBOX(loexp.message,48,_screen.caption)
ENDTRY 


SELECT (lcCurCom)
LOCATE
BROWSE 

FUNCTION Column2Field(tcCell)
	LOCAL lnLen, lnColumnStart, lcRetCol, lnCol
	IF PCOUNT() = 0
		RETURN ''
	ENDIF
	lcCell = UPPER(ALLTRIM(tcCell))
	lcRetCol = '' && which column to return (Col1, Col2, etc..)
	lnColumnStart = 64 && Since A = 65
	lnCol = ASC(lcCell) - lnColumnStart
	IF lnCol >= 1
		lcRetCol = 'Col' + TRANSFORM(lnCol)
	ENDIF 
	RETURN lcRetCol
ENDFUNC



Ez Logic
Michigan
 
Interesting, not seen LOCATE used in that way before - but it looks very sensible here.

Regards

Griff
Keep [Smile]ing

There are 10 kinds of people in the world, those who understand binary and those who don't.

I'm trying to cut down on the use of shrieks (exclamation marks), I'm told they are not good for you.
 
I tend to use LOCATE instead of GO TOP.

Seems faster. Not sure it is true.



Ez Logic
Michigan
 
According to the help, that is wise with either deleted on or a filter - although I had not seen it before today!

Regards

Griff
Keep [Smile]ing

There are 10 kinds of people in the world, those who understand binary and those who don't.

I'm trying to cut down on the use of shrieks (exclamation marks), I'm told they are not good for you.
 
Again, many thanks.. I can't believe how fast this is importing. I just imported a HUGE excel file that usually took me 14min+, I timed it with seconds()

and now it took ~7seconds

Jeez!

I am happy!

Ez Logic
Michigan
 
Thanks Griff, indeed i do have Deleted ON. Though I rarely use filters. i like to use cursors instead, as much as i can and close them as soon as i am done with them.

Ez Logic
Michigan
 
Yes, that looks much better. The point is that the only Automation you are now doing is to convert the workbook to a CSV. Since that it outside the loop, it will clearly run very much faster.

In my own version, I didn't even convert the workbook to a CSV. I appended it directly into the FoxPro cursor (using TYPE XL5). I assume you need the CSV to get the column headers.

Mike

__________________________________
Mike Lewis (Edinburgh, Scotland)

Visual FoxPro articles, tips and downloads
 
Mike, i had to convert it to CSV, since the excel file is actually a new version of the excel, it is xlsx. and i couldn't do TYPE XL5



Ez Logic
Michigan
 
Good to hear this is working, but a couple of suggestions. There is a lot of code that sets several variables in a row like:

Code:
 m.ActDate 	= CTOT(UPPER(ALLTRIM(&lcActDateCol)))
		m.MDN		= UPPER(ALLTRIM(&lcMDNCol))
		m.Master	= UPPER(ALLTRIM(&lcMasterCol))
		m.MasterName    = UPPER(ALLTRIM(&lcMasterNameCol))
		m.Sub		= UPPER(ALLTRIM(&lcSubCol))
		m.SubName	= UPPER(ALLTRIM(&lcSubNameCol))
		m.Serial	= UPPER(ALLTRIM(&lcSerialCol))
		m.MRC		= STRTRAN(ALLTRIM(&lcMRCCol),[$],[])
		m.MRC		= STRTRAN(m.MRC,[,],[])
		m.MRC		= VAL(m.MRC)
		m.Rate	        = STRTRAN(ALLTRIM(&lcRateCol),[$],[])
		m.Rate		= STRTRAN(m.Rate,[,],[])
		m.Rate		= VAL(m.Rate)

		m.Amount        = STRTRAN(ALLTRIM(&lcAmountCol),[$],[])
		m.Amount	= STRTRAN(m.Amount,[,],[])
		m.Amount	= VAL(m.Amount)

While it looks pretty, almost every book or coding standard I've read says don't line things up that way. If you have to add lines later, you'll spend lots of time lining things up.

I also see this

Code:
FUNCTION Column2Field(tcCell)
	LOCAL lnLen, lnColumnStart, lcRetCol, lnCol
	IF PCOUNT() = 0
		RETURN ''
	ENDIF

PCOUNT() is a programmer thing and serves no purpose for runtime. It gets run several times for no reason, adding processing time.

Craig Berntson
MCSD, Visual C# MVP,
 
Thanks Craig, I do agree with the lining up things. However, it makes it easier for me to "debug" when i visually look at the code. I am not a neat freak. but, i like indentations and code alignment as much as i can. Again, it is just to make it look good to easily spot something.

I don't know, to me it looks much cleaner, better, and much easier to spot a bug/error/syntax bug, than this:
Code:
m.ActDate = CTOT(UPPER(ALLTRIM(&lcActDateCol)))
m.MDN = UPPER(ALLTRIM(&lcMDNCol))
m.Master = UPPER(ALLTRIM(&lcMasterCol))
m.MasterName = UPPER(ALLTRIM(&lcMasterNameCol))
m.Sub = UPPER(ALLTRIM(&lcSubCol))
m.SubName = UPPER(ALLTRIM(&lcSubNameCol))
m.Serial = UPPER(ALLTRIM(&lcSerialCol))
m.MRC = STRTRAN(ALLTRIM(&lcMRCCol),[$],[])
m.MRC = STRTRAN(m.MRC,[,],[])
m.MRC = VAL(m.MRC)
m.Rate = STRTRAN(ALLTRIM(&lcRateCol),[$],[])
m.Rate = STRTRAN(m.Rate,[,],[])
m.Rate = VAL(m.Rate)
m.Amount = STRTRAN(ALLTRIM(&lcAmountCol),[$],[])
m.Amount = STRTRAN(m.Amount,[,],[])
m.Amount = VAL(m.Amount)

Ez Logic
Michigan
 
Status
Not open for further replies.

Part and Inventory Search

Sponsor

Back
Top