Home » HTTP » HTTP_Upload » Bug #2099

[2004-08-10 01:31 UTC] matthewh at meta-bit dot com

Description:
------------
Tomas,

I’ve been using your PEAR::HTTP Upload class recently for a Web/FTP based upload application.

I wanted to make several suggestions to the structure of the code.

My prime qualm is in the realm of internationalization [i18n] of your error strings. Within your source code you suggest actually altering your sourcecode to add additional languages but this is a very brittle solution.

We intend to support double-byte Asian languages such as Japanese and in the near future, Chinese, and as such, making wholesale changes to your sourcecode is very arduous each time a new patch of your library is released [we’re currently using v1.38 from the PEAR CVS repository].

Reproduce code:
---------------
The portion of your source code is around line 94 of HTTP Upload.php:
function HTTP_Upload_Error($lang = null, $html = false)
{
..........
// XXXXX Add here error messages in your language
$this->error_codes = array(
'TOO_LARGE' => array(
'es' => "Fichero demasiado largo. El maximo permitido es: $maxsize bytes.",
..................
),
'MISSING_DIR' => array(
'es' => 'Falta directorio destino.',
................
),
'IS_NOT_DIR' => array(

Expected result:
----------------
Ideally, HTTP_Upload_Error should use an external configuration file for _all_ error messages or otherwise use another PEAR library such as the otherwise excellent Translation2 library. This would mean, that all of our internationalized strings are centrally located within the Translation2 repository, rather than scattered between sourcefiles, flatfiles and databases.

One of the tenets of Object oriented programming is that you encapsulate entities that are likely to change regularly into their own object. Error messages most certainly fall into this category, and really should be externalised into a UTF-8/text based error message Resource file.

-----
As a further aside, I'd like to suggest that you don't use hardcoded strings for tokens within your source code but rather get into the habit of using define's.
eg: define( 'MISSING_DIR', 'missing_dir');

People would then refer to your defined symbols rather than duplicate hardcoded strings. It sounds like a lot of work, but you don't have many different types of error codes, and it reduces the likelihood of errors when people are using your library because PHP is checking the symbols.

Actual result:
--------------
Having hardcoded strings within your library means:
1. It's unworkable for us to add error strings and retro-fit them to each release you make in CVS.
2. Other people from the PEAR/PHP community can't readily contribute to your library, offering multi-lingual string translations without repeated releases of the _whole_ library.

[2004-11-26 00:42 UTC] zzz at zzz dot zz

I would like to see 1.0 version with this feature, one more thing to add, allowed/denied extensions are verified when file is already being moved, BUT, I want to check for errors before this, so I check for errors with isErrors, add data to mysql database, then call setName and moveTo. this needs a fix asap.

so to sum it all:
- better errors handling with i18n
- devide the class in parts. 1. uploading file 2. checking for errors here. 3. moveTo etc.

so basically, I want if(!$file->isError()) to include extension checks.. moving it away from moveTo function.