Re: Multiple File upload script

That was quite an upsetting piece of code to read. I have some new white hairs today :) I can't pinpoint your exact problem, you need to get down and dirty with debug, and may I suggest our old friend print could be put to good effect. Just from a once over, here's a handful of pointers. Firstly I think your files are zero length because they are never in the POST in the first place. I don't see an <input type=file ... anywhere in the form, or I'm not seeing how you get the data in there.

Upload Multiple and UploadMultiFiles are confusing use of two subs to do the job of one. You are passing parameters in a nasty way, flattening lists and then splitting them again to pass multiple parameters. Instead use a list, in the correct context a reference is automatically passed. There are some confusing constructs in there. Study the use of for and you will see that you can just say for(@array) without all those redundant indexes everywhere.

I think the whole thing would benefit from using a hash to hold all the datums, the path, the file, the associate and the description as keys. Copy these from the cgi query hash. You should be able to reduce the whole tangled pit of misery to 20 lines or less. Your problem with uploadInfo is that the $filehandles list doesn't contain a valid hashref, or $i is out of range.

Heres a few comments

10    require "settings.pl";                 # where is this?
12    umask(000);                    # this is asking for trouble
28    my $image = param('uniquecode');        # poor security, good as
+ useless
37-48    my ($tempError,$error,$name);            # you're using a sub
+ just to set a global
162    my ($paths,$newnames,$capts,$associate) = @_;    # passing in a
+ flattened list and expanding it is silly
163    my @filehandles = split(/\|/,$paths);        # use list ref ins
+tead and save loads of code
171    binmode $filenames[$i];                # operates on filehandle
+ not scalar
[download]

Best of luck Andy

Comment on Re: Multiple File upload script Download Code

Replies are listed 'Best First'.
Re: Re: Multiple File upload script by bionicle32 (Novice) on May 26, 2004 at 17:55 UTC
Andyf I have used the print statment within the uploadMutiFiles sub routine. I have all of the posted fullpaths from @filehandles as well as all of the posted captions from @descriptions. I removed the umask(000) from the script and can make a few adjustments to the code to accomodate the two variables being used from settings.pl. All they are is directory paths to a temporary directory and the housing directory if the file passes the uploaded byte test. I am sorry that the code is not the best you have seen, but we all get better over time. I could make the variables that I split global and that way they could be accessed by all subroutines. Is that what you were telling to do by using hashes? Any other comments would be useful. I will try to make some of those changes and let me know if you have further suggestions. Thanks, Shawn	[reply]
Re: Re: Re: Multiple File upload script by andyf (Pilgrim) on May 27, 2004 at 13:15 UTC
Shawn, If there is one thing you might take to heart I say it should be passing lists and hashes, a generaly more sophisticated way of going about things. No, I did not mean to suggest you use globals. The best analogy for functional programming is to imagine a production line with people all working a product that gets passed along. If you imagine each person is a suboutine/function then they get 'passed' some stuff, and they 'pass back/along' what they have done. What you are saying is... Bob, here's the spanner, Bob, heres the screwdriver, Bob, heres the hammer... Instead of just passing Bob the whole toolkit. If you pack all the items you want into a list, and pass the function a reference to that list, you effectively pass the whole bunch of stuff in one go. Of course the codes not bad, I jest. I've seen some _real_ horror in my time, if anything I see indications that you're a good coder limited by a few hangups. It's no worse than my perl code from not too long ago. Initially I thought you had not written the code yourself, and it was a cut n paste from a scripts site, but once I read it I realised you were struggling. I also see your background is in C, the move to Perl asks that you 'unlearn' some stuff. It's very forgiving too, and what looks syntactically nice to a C hacker is sometimes ugly in Perl. If you don't already have it I strongly recommend the Ram Book (cookbook), meshes perfectly with your level and need for practical snippets. have fun Andy	[reply]