Arnaud Limbourg [2005-11-21 12:07 UTC] As I said on the qa list I don't understand why a BC is needed. The same package with a property to set replace or search will have the same effect. Or do I miss something ?
Marco Kaiser [2005-11-21 12:16 UTC] I agree with Arnaud. It is not required to break BC and its much more better to extens the existing package.
anatoly techtonik [2005-12-03 19:43 UTC] Because in current state this package is unsafe to use - you may easily alter (hurt) your sources and other files in non-reversible way. Default behavior should be readonly.
anatoly techtonik [2005-12-04 11:00 UTC] To make it more clear - consider a new developer want to use a package. Package name indicates it's purpose - at the moment SearchReplace means only Replace operation, but from new release it will gain separate Search and Replace functionalities. So, when a new developer looks at API - (s)he sees two methods - doReplace() and doSearch(). Which one will be called to test replacement rules? I guess doSearch(), but it is easy to forgot to call it with necessary argument different from default to avoid replacements. Thus one can easily corrupt information in important files (including current sources).
Another side - try to read the text of the package, which uses File_SearchReplace:
<code>
include('File/SearchReplace.php');
$files = glob("*.php");
$search[] = "sion 3.0 of the PHP lic";
$replace[] = "sion 3.01 of the PHP lic";
$search[] = "license/3_0.txt";
$replace[] = "license/3_01.txt";
$snr = new File_SearchReplace( $search, $replace, $files);
$snr -> doSearch(false) ;
</code>
What the heck is this "false" search - does this mean replace will not happen or vice versa? doSearch() can be changed to require "true" to be passed, but the question is still the same - why doSearch(true) does replace and doSearch() does the same when there is doReplace() method what does the same?!
BC break is a matter of common sense. New "only search" functionality requires API to be changed. I proposed File_SearchAndReplace at first, but was convinced to continue as File_SearchReplace2.
Because of these BC requirements and proposal necessity package was delayed for at least half a year.
Arnaud Limbourg [2005-12-04 17:13 UTC] If it is documented that the search occurs then the hurt is done consciously by people who use it.
anatoly techtonik [2005-12-04 19:11 UTC] By default replace occurs. It is what should be changed.
Arnaud Limbourg [2005-12-05 06:10 UTC] The default should not be changed. Make a different behaviour optional.
anatoly techtonik [2005-12-05 21:00 UTC] Ok. Enough chatting. Let's vote. In any case this will be painless. So, the question is - for File_SearchReplace package, which will provide separate Search capability and Replace capability which API do you prefer?
vote +1 if : doReplace() must do search and replace and doSearch() must do only search
vote 0 if : you don't care
vote -1 if : you would like to use doSearch() for search and replace and doSearch(withparam) to do only search
|
