{"action":"created","issue":{"url":"https://api.github.com/repos/cakephp/cakephp/issues/9759","repository_url":"https://api.github.com/repos/cakephp/cakephp","labels_url":"https://api.github.com/repos/cakephp/cakephp/issues/9759/labels{/name}","comments_url":"https://api.github.com/repos/cakephp/cakephp/issues/9759/comments","events_url":"https://api.github.com/repos/cakephp/cakephp/issues/9759/events","html_url":"https://github.com/cakephp/cakephp/pull/9759","id":189090976,"number":9759,"title":"Refactor into smaller chunks of single responsibility methods.","user":{"login":"dereuromark","id":39854,"avatar_url":"https://avatars.githubusercontent.com/u/39854?v=3","gravatar_id":"","url":"https://api.github.com/users/dereuromark","html_url":"https://github.com/dereuromark","followers_url":"https://api.github.com/users/dereuromark/followers","following_url":"https://api.github.com/users/dereuromark/following{/other_user}","gists_url":"https://api.github.com/users/dereuromark/gists{/gist_id}","starred_url":"https://api.github.com/users/dereuromark/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dereuromark/subscriptions","organizations_url":"https://api.github.com/users/dereuromark/orgs","repos_url":"https://api.github.com/users/dereuromark/repos","events_url":"https://api.github.com/users/dereuromark/events{/privacy}","received_events_url":"https://api.github.com/users/dereuromark/received_events","type":"User","site_admin":false},"labels":[{"id":60504144,"url":"https://api.github.com/repos/cakephp/cakephp/labels/Enhancement","name":"Enhancement","color":"bfe5bf","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":{"url":"https://api.github.com/repos/cakephp/cakephp/milestones/111","html_url":"https://github.com/cakephp/cakephp/milestones/3.3.9","labels_url":"https://api.github.com/repos/cakephp/cakephp/milestones/111/labels","id":2117772,"number":111,"title":"3.3.9","description":"Bugfixes and minor features for 3.3.x","creator":{"login":"markstory","id":24086,"avatar_url":"https://avatars.githubusercontent.com/u/24086?v=3","gravatar_id":"","url":"https://api.github.com/users/markstory","html_url":"https://github.com/markstory","followers_url":"https://api.github.com/users/markstory/followers","following_url":"https://api.github.com/users/markstory/following{/other_user}","gists_url":"https://api.github.com/users/markstory/gists{/gist_id}","starred_url":"https://api.github.com/users/markstory/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/markstory/subscriptions","organizations_url":"https://api.github.com/users/markstory/orgs","repos_url":"https://api.github.com/users/markstory/repos","events_url":"https://api.github.com/users/markstory/events{/privacy}","received_events_url":"https://api.github.com/users/markstory/received_events","type":"User","site_admin":false},"open_issues":36,"closed_issues":23,"state":"open","created_at":"2016-11-07T02:36:42Z","updated_at":"2016-11-15T08:41:04Z","due_on":"2016-11-20T00:00:00Z","closed_at":null},"comments":4,"created_at":"2016-11-14T11:37:18Z","updated_at":"2016-11-15T09:25:19Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/cakephp/cakephp/pulls/9759","html_url":"https://github.com/cakephp/cakephp/pull/9759","diff_url":"https://github.com/cakephp/cakephp/pull/9759.diff","patch_url":"https://github.com/cakephp/cakephp/pull/9759.patch"},"body":"Refs https://github.com/FriendsOfCake/crud/pull/432#issuecomment-260230331\r\n\r\nFully BC"},"comment":{"url":"https://api.github.com/repos/cakephp/cakephp/issues/comments/260589913","html_url":"https://github.com/cakephp/cakephp/pull/9759#issuecomment-260589913","issue_url":"https://api.github.com/repos/cakephp/cakephp/issues/9759","id":260589913,"user":{"login":"dereuromark","id":39854,"avatar_url":"https://avatars.githubusercontent.com/u/39854?v=3","gravatar_id":"","url":"https://api.github.com/users/dereuromark","html_url":"https://github.com/dereuromark","followers_url":"https://api.github.com/users/dereuromark/followers","following_url":"https://api.github.com/users/dereuromark/following{/other_user}","gists_url":"https://api.github.com/users/dereuromark/gists{/gist_id}","starred_url":"https://api.github.com/users/dereuromark/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dereuromark/subscriptions","organizations_url":"https://api.github.com/users/dereuromark/orgs","repos_url":"https://api.github.com/users/dereuromark/repos","events_url":"https://api.github.com/users/dereuromark/events{/privacy}","received_events_url":"https://api.github.com/users/dereuromark/received_events","type":"User","site_admin":false},"created_at":"2016-11-15T09:25:19Z","updated_at":"2016-11-15T09:25:19Z","body":"@lorenzo Why would you think that it matters what submethods throws the exception?\r\nAll that matters is that the message is clear, then the stack trace is also there to clearly support what is going on.\r\n\r\nThe problem with not splitting is totally obvious: This method does too much.\r\nIt is way too long and people seem to have to use (2+ already have voiced that) copy and paste of that condition building outside instead of being able to use that core method.\r\nSee the crud PR that is referenced.\r\n\r\nThis method has too many things that it does:\r\n- building the conditions from primary key in a highly complex way\r\n- doing the actual finder call (what the primary task of this method is)\r\n- caching the result if necessary\r\n\r\nFrom a clean code perspective there is nothing better than splitting this up. Methods should have a single responsibility and not mixing multiple tasks.\r\nMaybe we could change the exception message if that helps?"}}
{"actor":{"display_login":"dereuromark"}}