davej / angular-classy

Cleaner class-based controllers with Angular 1
http://davej.github.io/angular-classy/
813 stars 27 forks source link

Confusing signature of method 'hasPrivateMethodPrefix' in 'addFnsToScope' plugin #22

Closed juangabreil closed 10 years ago

juangabreil commented 10 years ago

Hi,

First of all, congratulations for this awesome project! The code is very elegant and powerful!!

I've been reading your code in develop because I'm researching how to extend classy to use it in directives and services and I found something that looks confusing for me.

In line 243 the method hasPrivateMethodPrefix is returning true when the method name does not have the prefix and false when it does, the code is working because in line 253 you are checking if the method has the prefix (but should the opposite) to add it to the scope.

The fix is very simple, change != by == in hasPrivateMethodPrefix and add 'not' before calling this method. I can do a pull request, if you don't mind, or you can change it (if you want of course)

Regards

juangabreil commented 10 years ago

I've also detected that in 'classy-watch' plugin you check if $scope is injected in order to be able to use watch. I think you should do the same in 'classy-addFnsToScope' when you try to add public methods to scope.

Does it make any sense?

Regards

davej commented 10 years ago

This is great feedback and very useful, thanks! I'm away on holidays at the moment but when I'm back I'll take a look through the code and make some changes.

davej commented 10 years ago

Thanks again for the feedback, it's great to have someone go through my code and suggest fixes/changes. I'll make the commits to the develop branch over the weekend. :-)