amatriain / feedbunch

A simple and elegant feed reader.
http://feedbunch.com
MIT License
84 stars 11 forks source link

Remove private call on FolderManager as those methods are being called elsewhere #11

Closed yogodoshi closed 8 years ago

yogodoshi commented 8 years ago

The methods defined below the private definition aren't really private as they are being called in other files. On User#move_feed_to_folder for example.

yogodoshi commented 8 years ago

To fix the bundler-audit warnings: #13

amatriain commented 8 years ago

Where do you see those methods being called from outside their classes?

S3Client.key is a private method to calculate the key of an S3 asset. It is not called from any other class.

FolderManager.move_feed_to_folder is a public method that is called from User#move_feed_to_folder when a user requests moving a feed to a folder. However this method doesn't do the heavy lifting, it delegates to other private methods in the class; e.g. if the folder to which the feed is being moved already exists, it calls FolderManager.move_feed_to_existing_folder. The private methods of FolderManager are only invoked from methods in the same class.

amatriain commented 8 years ago

However, looking at this I've realized that while those class methods are supposed to be private, they really aren't because a private_class_method line is missing in those classes (and the private line does not really affect class methods). Thanks for pointing this out.

yogodoshi commented 8 years ago

@amatriain my explanation of "are being called in other files" wasn't right. Actually if you set these methods to private with private_class_method and run the unit tests you will see they will fail with:

NoMethodError: private method `remove_feed_from_folder' called for FolderManager:Class

amatriain commented 8 years ago

I'm closing this, those methods should indeed be private. There is a related issue, which is that private class methods were not declared correctly; this is addressed in PR #12