Tetsuji Koyama [2006-03-03 10:44 UTC] Why it depend on mbsting?
I found this code in several place:
mb_internal_encoding('euc');
Hatena API based on UTF-8, why you use 'euc'?
If someone use this classes with 'UTF-8' mb_internal_encoding, is it works fine?
Makoto Tanaka [2006-03-03 17:52 UTC] Thank you for advice.
And I think you are right.
I donot have to use "mb_internal_encoding".
Now, I revised it.
Thank you.
bertrand Gugger [2006-03-03 20:17 UTC] I cannot appreciate the uses of it, I will trust you on that.
PEAR has some CS == Coding Standards, you find them on the site,
for example, you may not write:
function blah() {...}
but you need to do:
function blah()
{
...
}
It's no bloody constraint, just get it more readable by any dev.
There's more, better you go through them first.
bertrand Gugger [2006-03-03 20:27 UTC] Very important, you may only require/include against fixed pathes.
It's the implementer fixing where stuff comes from.
Don't
require_once(HATENA_BASEDIR . "base.php");
but something like:
require_once 'Services/Hatena/base.php';
That's not to get it readable but secure and trustable.
Makoto Tanaka [2006-03-04 03:19 UTC] Thank you for advice.
I'm sorry : My code was not readable.
I revised it according to your advice.
--
(i) function blah() {...}
(ii) require_once( ... )
--
Thank you,
bertrand Gugger [2006-03-04 11:10 UTC] Looks very nice and complete. Few points more:
* you don't neet parenthesis in require as:
require_once 'Services/Hatena/base.php';
* use better single quotes everywhere you don't need some expansion, it's more performant
* you use PEAR but I don't find any require for it ?
* the rule by PEAR is one class per file, you may not declare 2 classes in Base.php, you need do separate Services_Hatena_login in its own file.
* you must put the first letter of coumpounds in uppercase as Services_Hatena_Login, file names also as Login.php
* one docbook entry missing for $parameters in Bookmark.php, Foto.php ...
* @version CVS: $Id$ only in file header, class header should have @version Release: @package_version@
* make a better description in package.xml, that will be on site and should explain what does the package. The thanks you have currently there would be better welcome in notes
* be aware version numbers as 0.0.x are only for the proposal time, your first release will have to be 0.1.0
* your test/demo hatena_services.php should be in doc/examples, not in data
* you are missing some dependencies (HTTP_Request, XML_RPC ...) in package.xml
* is it not possible to set the hatena base url in Base.php and have each service just add their own path to it ? you repeat it in each of them...
* also some services looks very similar they could eventually have share some code, but it's esthetic, not necessarly a problem
* More generally, I wonder if some unification would not be possible, each service has its own different interface, lot to learn ... but it's inherent to each service's particularity certainly.
* We tend to privilagiate PHP5, however you don't seem to necessitate it, being compat PHP4/5 could profit to more people ... (my personal opinion)
Makoto Tanaka [2006-03-05 05:27 UTC] I thank for very detailed advice very much.I am careful from the next time.
At first I revised it from the thing which was easy to wrestle.
Follows:
--
(1) removed parenthesis for require_once
(2) used single quotes for require_once
(3) required 'PEAR.php'
(4) separated file to Login.php
(5) renamed all class and file name.
asin.php to Asin.php and
Services_Hatena_asin, Services_Hatena_Asin
(6) removed $parameters in Foto.php and Bookmark.php
(7) used Release: @package_version@ for class comment
(8) changed description in package.xml
(9) moved from
data/hatena_services.php to doc/examples/Hatena_Services_
Example_01.php
(10) added dependencies HTTP_Request and XML_RPC in package.xml
--
And I changed Dependencies in proposal-show for HTTP_Request.
Thank you!!
Makoto Tanaka [2006-03-08 04:43 UTC] Every time, I thank for very detailed advice very much.
>is it not possible to set the hatena base url in Base.php
I revised it.I set base url in Base.php, and used it from Bookmark.php, Foto.php and etc..
I want to examine correspondence to PHP4 and commonization in future.
--
Thank you,,
|
