Import & sorting issues 2

[Mrall]

The import works but lots of blank records are created in the process. I'm not sure why. I also figure there must be a more effective way to do what I am trying to do. This is a import routine called by a button in a form.

imdata = GETFILE("xls")
filpth = trim(imdata)
***********************************************************************
*    Do the following process only if a file is selected              *
*    If cancel is selected than this process is skipped               *
***********************************************************************
if len(filpth) > 1 THEN 
  thisform.Pageframe1.page2.text3.Value = filpth
  SET EXCLUSIVE ON
  SET SAFETY OFF
  **** Empty imptemp ***
  SELECT imptemp
  ZAP
  *********************************************************************
  *    Progress bar to make it look cool                              *
  *********************************************************************
  thisform.Pageframe1.page2.Text4.value = "Importing Spreadsheet"
  thisform.Pageframe1.Page2.Olecontrol1.Visible = .t.
  For I = 1 TO 25
		for x= 1 to 40000
		next x
		Thisform.Pageframe1.Page2.Olecontrol1.value = ((I*100)/100)
		x=0
  Next I 
  *********************************************************************
  *     Import Spreadsheet                                            *
  *********************************************************************
  append from &filpth TYPE XL8 SHEET "6A MACD List"
  **** The first record in the imported file is the field names **
  **** so it needs to be deleted **
  GO top
  DELETE record 1
  PACK
  *********************************************************************
  *        Need to place the job imformation into the jobs table      *
  *********************************************************************
  SELECT jobs
  APPEND BLANK
  replace projname WITH TRIM(imptemp.customer)
  replace address WITH TRIM(imptemp.address)
  replace city WITH TRIM(imptemp.city)
  replace zip WITH TRIM(imptemp.zip)
  replace state WITH TRIM(imptemp.st)
  *********************************************************************
  *    Sort imptemp to temp2 and erase the records and put            *
  *    the sorted records back into imptemp and delete temp2          *
  *********************************************************************
  SELECT imptemp
  SORT TO temp2 ON dispositio, xsm_site_n
  ZAP
  APPEND from temp2
  ERASE temp2
  *********************************************************************
  *    For each record in imptemp create a record in disposition      *
  *********************************************************************
  FOR b= 1 TO RECCOUNT()
  	GOTO RECORD b
  	*******************************************************************
  	*            Part of the progress bar                             *
  	*******************************************************************
  	thisform.pageframe1.page2.text4.Value = "Importing Record "+STR(RECNO())
  	
  	
  	SELECT dispositions
  	APPEND BLANK
  	replace projname WITH TRIM(imptemp.customer)
  	replace xsmsite WITH TRIM(imptemp.xsm_site_n)
  	replace disposition WITH TRIM(imptemp.dispositio)
  	
  	DO CASE
  	  CASE TRIM(disposition) = "Add"
  					replace finalstatus WITH "Pending" 
  	  CASE TRIM(disposition) = "Out of scope"
  					replace finalstatus WITH "NA"
  	  CASE TRIM(disposition) = "Move"
  					replace finalstatus WITH "Pending"
  	  CASE TRIM(disposition) = "Remove-Regency"
  					replace finalstatus WITH "Pending"
  	  CASE TRIM(disposition) = "Remove-Depot"
  					replace finalstatus WITH "Pending"
  	  CASE TRIM(disposition) = "Remove-Donate"
  					replace finalstatus WITH "Pending"
  	  CASE TRIM(disposition) = "Unchanged"
  					replace finalstatus WITH "Pending"
  	ENDCASE
  	 
  	replace disdate WITH TRIM(imptemp.dispositi1)
  	replace assetno WITH TRIM(imptemp.asset_numb)
  	replace manufacturer WITH TRIM(imptemp.manufactur)
  	replace model WITH TRIM(imptemp.model)
  	replace serialno WITH TRIM(imptemp.serial_num)
  	replace orgdept WITH TRIM(imptemp.org___dept)
  	replace movefrom WITH TRIM(imptemp.move_from_)
  	replace roomcube WITH TRIM(imptemp.room___cub)
  	replace meterreading WITH TRIM(imptemp.meter_read)
  	replace contact1name WITH TRIM(imptemp.contact_na)
  	replace contact1number WITH TRIM(imptemp.contact_ph)
  	replace contact1email WITH TRIM(imptemp.contact_e_)
  	replace comments WITH TRIM(imptemp.comment)
  	replace faxan WITH TRIM(imptemp.fax__analo)	
  	replace rightfax WITH TRIM(imptemp.fax_number)
  	replace ipaddress WITH TRIM(imptemp.ip_address)
  	replace gateway WITH TRIM(imptemp.gateway)
  	replace subnet WITH TRIM(imptemp.subnet)
  	replace macaddress WITH TRIM(imptemp.mac_addres)
  	replace printername WITH TRIM(imptemp.printer_na)
  	SELECT imptemp
  NEXT b
  
  ************************************************************
  *          Progress Bar                                    *
  ************************************************************
  For I = 26 TO 100
	for x= 1 to 40000
	next x
	Thisform.Pageframe1.Page2.Olecontrol1.value = ((I*100)/100)
	x=0
  Next I
	
	
	else
endif

SET SAFETY ON
SET EXCLUSIVE OFF
thisform.Pageframe1.page2.Text4.value = ""
thisform.Pageframe1.Page2.Olecontrol1.Visible = .f.
thisform.Pageframe1.page2.Text3.visible = .f.
thisform.Pageframe1.page2.Command9.visible = .f.
thisform.Pageframe1.page2.Label4.visible = .f.
thisform.Pageframe1.page2.Refresh

 

[Mike Lewis]

It would be helpful if you could figure out at which point in the code the problem arises.

You say there are blank records in the file. You should step through your code in the debugger, examining the output file and intermediate files at various points, to see exactly where these blank records are first appearing.

Also, it's difficult for us to debug your code without knowing more about the contents of the XLS file. For all we know, there might be blank rows within the file.

Mike

__________________________________
Mike Lewis (Edinburgh, Scotland)

Visual FoxPro articles, tips, training, consultancy

 

[MrDataGuy]

Some suggestions on the code:
(BTW None of these comments are the root cause of the blank records, but I would require my programmers to fix these side issues before I would spend a lot of time tracking down the 'real bug'. Having clean code (my definition of clean code, which of course is not everyone's ) is step one on debugging. )

-Variables are being used (X, I, B ) that have not been declared LOCAL, bugs may result.

-Setting SAFETY OFF and leaving it off for a long time IMO that is not good idea, turn it back on ASAP

-Current Code:
if len(filpth) > 1 THEN
…Lots of code
Endif

IMO Better is to do

If len(filpth) > 1 THEN
Return ‘<<<<<<< Quick Exit
Endif

…Lots of code

-Looks to me that the progress bar ‘movement’ is a complete ‘fake’. The code is just looping wasting time, not really telling the user what is going on. To be frank, looks to me like unneeded/unwanted/wasted code.

-It is unclear ( to me at least ) what table/cursor the file is being appended to (Jobs or imptemp maybe? ). Instead of appending to an existing table, might it be better to create a cursor, append it to that, do any data cleaning to the data in the cursor, then append the new data to the table?

-Instead of a SORT command, what about using a SQL SELECT w/SORT. No need to erase a temp file (Much Faster if that matters>0

-A bigger issue if that this is a mixture of Business Logic (Importing of a file ) and UI (Updating a Progress Bar ). IMO (OK Some/Many/Most VFP Developers may say I am going to far) this code needs to be separated. There should be a Business Object Class created to do the Importing, with a percentage done property, and the UI calls that Biz Class and with a timer every so often asks the biz Class how far along the processing is, and then updates a progress bar. IMO this current code is doing too much. Each ‘bit’ of code should do one and only thing. This code is too complex. Again that is my opinion.


Lion Crest Software Services
Anthony L. Testi
President

http://www.LionCrestSoftwareServices.com

 

[Mike Lewis]

A couple of other points:

- Your first block of REPLACE commands is definitely suspect. Because these commands don't have a SCOPE or FOR clause, they will only replace values in the new record, from the current record in Imptemp. But since you have just packed that table, you can't know for sure which record it is sitting on.

- Your FOR loop is also a bit strange. Why not just SCAN the table?

- Your whole sorting routine is convoluted and time-consuming. Just create an index on the table.

If'm not saying that any of these issues is the cause of your problem, but it would simplify your code if you dealt with them - and therefore make it easier to debug.

Mike

__________________________________
Mike Lewis (Edinburgh, Scotland)

Visual FoxPro articles, tips, training, consultancy

 

[GriffMG]

Well my first thought is that your progress bars are not really demonstrating progress - they seem to be outside of the loop that
processes your import.

The next thing I see is the use of a macro substitution to access the excel file in question - I would recommend that you use this format instead:

append from (filpth) TYPE XL8 SHEET "6A MACD List"

It allows for spaces in the paths and file names.

Next, you have used a sort command, when you would probably do better with an index and instead of a For... Next loop to process the table, I would suggest a Do While... skip... enddo or a simple Scan... endscan construct.

If you want avoid the empty records in the output file, I would test two things; firstly I would use an If !empty(imptemp.customer)... endif construct around your addition and processing of new records and secondly I would perhaps not add any records which fell outside your values specified in your Do Case... endcase criteria.

Good luck

Regards

Griff
Keep [Smile]ing

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

 

[Dan Freeman]

On coding style, alone, I agree with everything said here and would add that multiple replace statements as you have them will make this code GLACIALLY slow. Combine them into a single command, i.e.:

replace disdate WITH TRIM(imptemp.dispositi1), ;
        assetno WITH TRIM(imptemp.asset_numb)

As written, I'd say the blanks are most likely coming from the import file. Garbage in, garbage out.

 

[Mrall]

You are right about the progress bar it isn't really needed at all. The process is so fast that it's done almost as soon as the you finish clicking the file name. I just wanted to user to know the process was being done (at a speed they could see). I could just put a message at the end that states that so many records were imported successfully instead. I could have added code in for the progress bar as each of record were added as well.(that way it would not be a fake)but without the delays it goes so fast you never see the progress bar anyway. I just don't like clicking a button and not knowing if the process is started, done or even if it completed successfully. I also, thought that if I put the progress bar in and it errored out I might be able to tell about where the problem occured. (my error routine work great) Maybe I'll just remove it.

This is a very small program that deals with only two tables, one form, and two reports. All the input comes from the imported Excel spreadsheet. No records are added and only certain fields can be edited. No need for fancy business object classes or extra PRGs with lots routines that won't be used. With the exception of my well written main.prg, the one Form is the entire program. When you exit the form the program is done.

Thanks for all the help

I'm going to index the table and use the scan to make things easier. I believe the blanks are coming from the excel file and I'll just filter them out.

 

[MrDataGuy]

Why not just show a message box at the end, or update some information on the screen to tell/show the user the results of the input.

e.g. "Finished updating information, 25 records added!"

Lion Crest Software Services
Anthony L. Testi
President

http://www.LionCrestSoftwareServices.com