Upload file and use for extraction 3

[TheLazyPig]

Hi!

I'm going to create a form that uploads a single .dbf file and use it as a table for extraction.

Below is the form layout...

textbox = disabled; views the file name uploaded
Upload = should get the .dbf file in the local driver of the user
Extract = after upload, extract the file using SELECT

I used GETFILE() to get the file and put the filename in the textbox.

Upload File Button

CLOSE DATABASES
SELECT 0

LOCAL gcFile

gcFile = GETFILE('DBF','Browse','Browse')

thisform.txtFileName.Value = JUSTFNAME(gcFile)

IF !EMPTY(ThisForm.txtFileName.Value)
   thisform.btnExtract.enabled = .T.
ENDIF

Extract Button

SET CENTURY OFF
SET CENTURY ON
SET DELETED ON
SET SAFETY OFF
SET UDFPARMS TO REFERENCE

CLOSE ALL
CLEAR

LOCAL lcDir, lcFile

lcTest   = "'C:\RECOMPUTE\RECOMPUTE2\'"
lcDir    = gcFile
lcFile   = "'C:\BACKUP_FILES\AGLIB_EXTRACT\'"

USE lcDir IN 0 SHARED NOUPDATE 
	 
SET DELETED ON
			
SELECT * FROM AGLIB WHERE rec_status <> 'D'
 	
IF MESSAGEBOX("Extract Agent Library?",36,"Confirmation") = 6 
   COPY TO lcFile+"aglib_extract" TYPE XL5
   MESSAGEBOX("File [aglib_extract.xls] Created!")
	      
ELSE 
   MESSAGEBOX('Extraction Cancelled')
   RETURN 0
ENDIF

However, I get an error from gcFile. It can't locate the local variable that I used from the upload button.
The location and name of the file uploaded should be the path (set default to) of the extraction.

Thank you!!!

 

[Mike Lewis]

There are quite a few things here that need fixing. But to focus on this particular error, the problem is that you are declaring gcFile in the click of the Upload button, but trying to use it in the click of the Extract button. Given that it is a local variable, it is only visible in the method in which it is created.

The solution here is to use the value that you have already stored in your textbox. So instead of this:

lcDir = gcFile

Do this:

[tt]lcDir = ALLTRIM(thisform.txtFileName.Value)[/tt]

Also, in your USE statement, you will need to put parens round the file name:

USE [highlight #FCE94F]([/highlight]lcDir[highlight #FCE94F])[/highlight] IN 0 SHARED NOUPDATE

otherwise it would look for a file whose actual name is lcDir.

I suggest you focus on fixing this, and then we can talk about some of the other points that need attention.

Mike

__________________________________
Mike Lewis (Edinburgh, Scotland)

Visual FoxPro articles, tips and downloads

 

[TheLazyPig]

Okay, I fixed it. But lcDir is only the filename.

 

[Mike Lewis]

What do you mean by "lcDir is only the filename". lcDir is not the filename; it is a variable that holds the filename.

So is it working now?

Mike

__________________________________
Mike Lewis (Edinburgh, Scotland)

Visual FoxPro articles, tips and downloads

 

[Mike Lewis]

What do you mean by "lcDir is only the filename".

Ah! I guess you mean that lcDir contains just the stem of the filename, without the path or extension. Is that right?

If so, a better solution would be to create a custom property of the form to hold the full path and filename. Then, in the Upload button, you would store the result from the GETFILE() (which includes the full path and extension) in that property. And in the Extract button, you would use the contents of the property in the USE command - and don't forget the parenthese.

Mike

__________________________________
Mike Lewis (Edinburgh, Scotland)

Visual FoxPro articles, tips and downloads

 

[Chris Miller]

You either replace this

thisform.txtFileName.Value = JUSTFNAME(gcFile)

with this

thisform.txtFileName.Value = gcFile

And then use thisform.txtFileName.Value as a replacement for a variable that still knows the user choice.

I wonder if it wouldn't be better to join the two buttons functions and make it choose AND upload the file.

Aside from that, you go by naming conventions VFP recommends, but you use them wrong, either you do a LOCAL variable lcFile and declare it LOCAL lcFile, or you want to use a global variable (not recommendable just because one other method of the form needs this information), then you'd name that gcFile, but the g indicating globale means you declare it with - wrong, not GLOBAL, but PUBLIC. there are several examples in the VFP help from which you can see this. The reason public variables don't get the prefix p for public is that the naming convention already uses p for private variables.

You mainly have three variable scopes LOCAL, PRIVATE, and PUBLIC. LOCAL does not work, as you know, PUBLIC would work but gives the variable a much too big scope, it should only be accessible by both form buttons, actually. The closest scope you have per existing object structure - is the form. PRIVATE won'T work, too. So instead of a variable, you use a property, which is quite the same as a variable, just scoped to a specific object it's a member of. And you can create such properties with code just like LOCAL or PUBLIC creates variables in code, if you think aboout it that way, this is not a compiler directive, this is actually a command to create variables. You can use object.AddProperty or in VFP9 the ADDPROPERTY function to create properties. It's really not strange to interprest properties as variables with a scope that's somewhere between LOCAL and PUBLIC. And you should always use the scope that's the best fit for all code that needs a value stored into it. So it's often properties that are the solution to be used as variables with a scope you need.

Any method should normally only need access to values in variables it creates itself, things passed in go as parameters and another valid source of data always is tables, of course. Current form controls and their VALUE property are usually not mainly there to exist as variables for your code, but as a display of data to the user. In this case, storing filename including the directory would be okay to me, though it takes more area on the form, than just the filename. And in this csse you don't store that to a table, you just keep it temporarily for the process you turned into two steps with the disabled button that's only enabled after the first step of picking a file is done.

In strict terms, you would create a form property as Mike suggested in his last post or make it a property of the upload file button, which you perhaps name pickedFile. Then the extract button could read it from that form or upload button property. Always think, whether it makes sense to create a dependency, in case you would delete the upload button, the whole process doesn't work, so it would be oka to introduce this dependency you could think of making the two buttons and textbox a class that's based on a container to be able to reuse it in further forms.

As said early on, I'd not make this a two step process, when it can be one step, which remoes the whole problem of vaiable scooe. But that's personal taste, perhaps. Nevertheless, if you do it in two steps, I'd not caption a button "upload", if it only picks the file, the actual upload is done by the extract button, as far as I get your description, but I don't get why you name that "extract". It's the actual upload or submit button, instead, isn't it?

Chriss

 

[Mike Lewis]

I wonder if it wouldn't be better to join the two buttons functions and make it choose AND upload the file.

Yes, I was going to suggest this myself. From the user's point of view, the selection and the uploading are two parts of the same operation, so why does it need two separate steps?

Mike

__________________________________
Mike Lewis (Edinburgh, Scotland)

Visual FoxPro articles, tips and downloads

 

[Mike Lewis]

Just a few other points with this code, off the top of my head:

- Its not clear why you've got those SET commands (SET CENTURY, etc) at the top of the second method. Apart possibly from SET DELETED (which, by the way, you appear to have duplicated), none of them is relevant to this particular code.

- Similarly with CLOSE DATABASES, SELECT 0, CLOSE ALL, CLEAR etc. In particular, CLEAR on its own does nothing but clear the background screen; why would you want to do that here? And you've got to be careful with CLOSE ALL. If you are using it to close your tables (which partly overlaps with CLOSE DATABASE), be aware that it closes other items as well, such as any open procedure files.

-Your 'Extraction Cancelled' message is unnecessary - and arguably intrusive. The fact that the user has just hit the Cancel button provides the information that the process was cancelled. They don't need to be told what they have just done.

- There is always a worry in hard-coding file and path names, as you have done when setting lcTest and lcFile. Are you sure those directories will always be present and always on the C drive? What if one day you need to move them to another system or a shared folder?

- Regarding the text box, it would be preferable to make it read-only rather than disabled. A read-only text box is not only easier to read (given the default colours), it also enables the user to scroll horizontally if the text is wider than the box, and also to copy the text to the clipboard. None of that might be important in this case, but is worth keeping in mind.

- I can see why you want to enable the Extract button only when the user has selected a filename. But have you thought what would happen if they do that twice? They might correctly select a filename the first time, thus causing the button to be enabled. But they then might hit the Upload a button a second time, and either fail to select a file or select an invalid file. But the Extract button will still be enabled, which is not what you want. In fact, it will remain enabled for the life of the form. A simple solution would be always to explicitly disable the button at the start of the method. (Of course, this won't apply if you combine the two routes, as suggested earlier.)

- And finally there are the questions of variable scope and naming conventions that Chris has already dealt with in detail.

LazyPig, I hope you won't take exception to my mentioning these points. None of them is vital to solving your original problem. But they are all important for tidy, logical programming.

Mike

__________________________________
Mike Lewis (Edinburgh, Scotland)

Visual FoxPro articles, tips and downloads

 

[Chris Miller]

I see another problem, when we're at pointing them out:

You do

lcFile   = "'C:\BACKUP_FILES\AGLIB_EXTRACT\'"

I can guess why you put single quotes into double quotes here, a reason would be using that in macro substituion, but still wanting the substitution to be a string and not the direct path.

But then you don't use macro substitution later but do:

COPY TO lcFile+"aglib_extract" TYPE XL5

Make yourself aware what lcFile+"aglib_extract" is. Sine lcFile is not changed by user interaction at all, it's cleary becoming the string

"'C:\BACKUP_FILES\AGLIB_EXTRACT\'"+"aglib_extract" = "'C:\BACKUP_FILES\AGLIB_EXTRACT\'aglib_extract"

And that's surely not what you want and need.

If you struggle with the effects of quotes and macro substitution and specifing file names in commands so you can use a variable as part of the file name, then make use of brackets. So very specifically in commands needing a file name (not functions) like COPY FILE, USE, DO FORM and many more, it never hurts to use brackets first.

So instead of

DO FORM myform

you start using the long form

DO FORM ("myform.scx")

And instead of

USE mytable

you start writing

USE ("mytable.dbf")

This way you get used to specifying any file name as such an expression. This can get varied towards something using n expression within the brackets.

COPY TO (lcBasepath+"defaultfilename.xls")

Am I serious about this? Well, we are very used to having tables in our SET PATH or even SET DEFAULT aka CD to the database folder to have it easy with any USE and only specify thee table name, not the dbf extension - that's automatic, not the path. This works, as we avoid spaces in dbf file names, which would work as file name, but are not allowed in alias names.

I don't say everybody should switch from USE mytable to USE ("mytable.dbf"), but if you switch programming it this way you get used to the pattern of how VFP will always evaluate something in brackets to a final value of an expression as a file name. an that expression can be however complicated as you need it to be, like (goApp.Databasedir+lcCustomerpath+"table.dbf"). It may even include function calls like (goApp.GetConfiguration("Databasefodler")+goApp.GetConfiguration("CustomerDataSubfolder")+"table.dbf").

I see a lot more to address in your code, I just don't want to mangle several pieces of advice into one post.

Chriss

 

[TheLazyPig]

Thank you for the replies.

- Its not clear why you've got those SET commands (SET CENTURY, etc) at the top of the second method. Apart possibly from SET DELETED (which, by the way, you appear to have duplicated), none of them is relevant to this particular code.]I followed your advice

I removed everything except the SET DELETED ON, Close Database from the upload button and 'Extraction Cancelled'. And changed my textbox to read-only

- I can see why you want to enable the Extract button only when the user has selected a filename. But have you thought what would happen if they do that twice? They might correctly select a filename the first time, thus causing the button to be enabled. But they then might hit the Upload a button a second time, and either fail to select a file or select an invalid file. But the Extract button will still be enabled, which is not what you want. In fact, it will remain enabled for the life of the form. A simple solution would be always to explicitly disable the button at the start of the method. (Of course, this won't apply if you combine the two routes, as suggested earlier.)

I did some changes to my code, I added a filter (ATC()) where if the file does not contain the word AGLIB will get an Invalid File Name error. Then I removed the value inside the textbox and disabled again the Extract button. I also added it after the file created.

"'C:\BACKUP_FILES\AGLIB_EXTRACT\'"+"aglib_extract" = "'C:\BACKUP_FILES\AGLIB_EXTRACT\'aglib_extract"]

I removed the single quotes from my paths

Overall, it works. But after I click the extract button the table will preview (pop-up) before the file extracts, is it really like that?


Thank you so much!!!

 

[Mike Lewis]

Good to hear that our advice has been helpful.

Regarding the "preview", I suspect this is a Browse window that is showing he result of your [tt]SELECT * FROM AGLIB WHERE rec_status <> 'D'[/tt] command. In general, a Browse window is the default destination for a SELECT. You can see this for yourself by doing any SELECT in the command window. It will cause a Browse window to open to show the results. The same applies when doing the SELECT in a program, as is the case here.

The solution is to specify a cursor to hold the result of the SELECT. Just add the words [tt]INTO CURSOR[/tt] at the end of the SELECT, followed by the name of the cursor.

Mike

__________________________________
Mike Lewis (Edinburgh, Scotland)

Visual FoxPro articles, tips and downloads

 

[Chris Miller]

the table will preview (pop-up) before the file extracts

I see Mike has already addressed that, it's the next thing to address in your code, but it will be very obvious to you, as we see you mentioned it. You should be aware that a query should always have an INTO clause, and since you want to create a file of the query result with COPY TO, it must be a cursor.

You could, by the way, also create a DBF directly from the query, if you make it INTO TABLE. Of course, then you get a DBF, not an XLS.

In this case it doesn't matter that you get the browse, it's just an annoyance, perhaps, but I was also not sure that you intended to show the data result to the user before creating the XLS that will look the same within the boundaries of how VFP and Excel display their data. This browse window you now suppress by using INTO CURSOR, as Mike told you, would also enable users to edit this data before it's put into the XLS. But they could do that anyway within the XLS file.

Chriss

 

[TheLazyPig]

You should be aware that a query should always have an INTO clause, and since you want to create a file of the query result with COPY TO, it must be a cursor.

Ok Sir I'll remember that.

I still want to add about saving the file. If it can choose what name to save like when you save as a file in excel. Or should I create another thread of this? hehe


Thank you so much, sir/s!

 

[Mike Lewis]

If it can choose what name to save like when you save as a file in excel.

The VFP equivalent is PUTFILE(). Like GETFILE(), it returns the full path and filename of the selected file; and it returns an empty string if the user cancels. Unlike GETFILE(), it warns the user if they choose a file that already exists (on the basis that would result in the file being overwritten).

Note that PUTFILE() doesn't actually save or copy a file. It merely returns the name of the file that the user picks.

Mike

__________________________________
Mike Lewis (Edinburgh, Scotland)

Visual FoxPro articles, tips and downloads

 

[Chris Miller]

Mike gave you the answer.

Notice, when you were reading about GETFILE() or GETPICT() in their help topics, you'd been pointed to PUTFILE() in the "See also" section. The VFP help makes you discover a family of commands or functions.

GETPICT() see also:

GETFILE() see also:

Een if you don't go there in the fear of getting carried away and eventally reading the whole help, you can know the names of similar functions of the same family of functions when just being pointed to one of them.

Chriss