Hi Greg, Am Freitag, den 28.03.2008, 17:13 -0500 schrieb Greg Beaver: [...] > In addition, this would mean that offsetGet() would have to create a > directory. Why? > > $phar['full']['path']['to']['file'] = 'hi' calls offsetGet() with > 'full', 'path', and 'to', and offsetSet() only with 'file'.
I know that and I discussed that with Marcus beforehand. I don't think that implicitly creating the directory is much an issue. If somebody does not want that, he can just use isset() before. > In other words, the proposed change is much more complex, introduces > magic and counterintuitive behavior inside the extension. For instance, > let's say we have this code: > > <?php > $a = $phar['dir']; > $b = $phar['dri']; // typo > ?> > > if phar.readonly=1, the second line would throw an exception for > attempting to create the directory "dri" Wouldn't the second line throw an exeception anyway because the index is not defined? And: can't offsetGet() check weither Phar::canWrite() is true? > What about this? > > <?php > unset($phar['dri']); // dri doesn't exist > ?> > > The above code would first create the dri directory, and then erase it. No, it would not. Please take a look at http://lars.schokokeks.org/php/phar.phps and http://lars.schokokeks.org/php/phar.php > In other words, the proposed syntax is not a good idea. Two of three assumptions were not correct or at least arguable, so I can't agree that easily. > 2) Names must be very carefully considered in relation to existing APIs > and the pre-existing methods. > > Phar::create(), for instance, should be named buildFromDirectory() to > match buildFromIterator(). I agree, that this name is much better. > what benefit is there in adding complexity of remembering the Phar::GZ > constant over setCompressedGZ()? We have to think hard about changes > like this. setCompressedGZ() is just not the way object oriented APIs should behave, as it taints the API with concrete implementation details ("there can be gzip compression and every compression algorithm is exposed" opposed to "there can be compression, one variant is gzip"). Phar::compress() indicates that "the archive can be compressed" with the concrete algorithm as a parameter. What about in 5 years when a new, free compression algorithm rises, which fixes the problems of bzip2. Would you like to add another setCompressedFooBar() or just add the algorithm and the class constant and be fine? If you insist on setCompressedFoo(), I would at least propose to not implement that as a setter but to describe the attached behaviour, which would be Phar::compressWithGzip() or something like that. Nevertheless, please seriously consider to do it with a single compress()-method. > 3) I would like to suggest these changes, and these alone: > > * simplify all compression methods, with more consistency of naming > that clearly differentiates between compressing files within an archive, > and compressing the entire archive. This abstract idea is more > important to me than the specific naming, but it should be very easy to do. What would be your concrete proposal regarding that? [...] cu, Lars
signature.asc
Description: Dies ist ein digital signierter Nachrichtenteil
