search

angeloskath / php-nlp-tools

Natural Language Processing Tools in PHP
Do What The F*ck You Want To Public License
745 stars 152 forks source link

Autoload script issues #1

Closed relwell closed 11 years ago

relwell commented 11 years ago

Hi there,

Your autoload script throws an exception when a class isn't found. This seems bad, because it can prevent other autoload functions from running. I understand exception handling when we don't have a given class is nice, but why wouldn't we just use class_exists() to check?

Thanks!

angeloskath commented 11 years ago

Hi,

Thanks a lot for the feedback, I appreciate it.

Regarding the issue, you have a point. I will change it. Do you think another kind of error reporting is more appropriate (actually your point is that it is not an error so probably no error reporting is better)?

In addition, this autoloader is only when not using composer.

Angelos

PS.: I will take at least a week to do this change because I am on vacation On Jul 10, 2013 8:18 PM, "Robert Elwell" notifications@github.com wrote:

Hi there,

Your autoload script throws an exception when a class isn't found. This seems bad, because it can prevent other autoload functions from running. I understand exception handling when we don't have a given class is nice, but why wouldn't we just use class_exists() to check?

Thanks!

— Reply to this email directly or view it on GitHubhttps://github.com/angeloskath/php-nlp-tools/issues/1 .

relwell commented 11 years ago

Hi Angelos,

I'd suggest adhering to the PSR-0 standards proposed by the Framework Interoperability Group. This also affects your directory structure. However, it's a best practice and makes your library easier to maintain when adding classes.

Robert

angeloskath commented 11 years ago

Thanks, for the suggestion. I agree, it is something I have been avoiding to do for quite a long time, although the sooner I did it the better. If you see commit dcde3a2fc599c3a9a5a0435243f953c4537ef5d3 I grouped the classes in conceptual groups each in their own namespace (Models, Tokenizers, etc) in order to enable me to transition to PSR-0 some time in the future.

I don't think I need to assign the issue to anyone, I will restructure the library and then close the issue.