{"action":"created","comment":{"url":"https://api.github.com/repos/OpenRA/OpenRA/pulls/comments/23957968","id":23957968,"diff_hunk":"@@ -143,145 +183,123 @@ public PathSearch FromPoint(CPos from)\n \t\t// For horizontal/vertical directions, the set is the three cells 'ahead'. For diagonal directions, the set\n \t\t// is the three cells ahead, plus the two cells to the side, which we cannot exclude without knowing if\n \t\t// the cell directly between them and our parent is passable.\n-\t\tstatic CVec[][] directedNeighbors = {\n-\t\t\tnew CVec[] { new CVec(-1, -1), new CVec(0, -1), new CVec(1, -1), new CVec(-1, 0), new CVec(-1, 1) },\n-\t\t\tnew CVec[] { new CVec(-1, -1), new CVec(0, -1), new CVec(1, -1) },\n-\t\t\tnew CVec[] { new CVec(-1, -1), new CVec(0, -1), new CVec(1, -1), new CVec(1, 0), new CVec(1, 1) },\n-\t\t\tnew CVec[] { new CVec(-1, -1), new CVec(-1, 0), new CVec(-1, 1) },\n+\t\tstatic readonly CVec[][] DirectedNeighbors = {\n+\t\t\tnew[] { new CVec(-1, -1), new CVec(0, -1), new CVec(1, -1), new CVec(-1, 0), new CVec(-1, 1) },\n+\t\t\tnew[] { new CVec(-1, -1), new CVec(0, -1), new CVec(1, -1) },\n+\t\t\tnew[] { new CVec(-1, -1), new CVec(0, -1), new CVec(1, -1), new CVec(1, 0), new CVec(1, 1) },\n+\t\t\tnew[] { new CVec(-1, -1), new CVec(-1, 0), new CVec(-1, 1) },\n \t\t\tCVec.Directions,\n-\t\t\tnew CVec[] { new CVec(1, -1), new CVec(1, 0), new CVec(1, 1) },\n-\t\t\tnew CVec[] { new CVec(-1, -1), new CVec(-1, 0), new CVec(-1, 1), new CVec(0, 1), new CVec(1, 1) },\n-\t\t\tnew CVec[] { new CVec(-1, 1), new CVec(0, 1), new CVec(1, 1) },\n-\t\t\tnew CVec[] { new CVec(1, -1), new CVec(1, 0), new CVec(-1, 1), new CVec(0, 1), new CVec(1, 1) },\n+\t\t\tnew[] { new CVec(1, -1), new CVec(1, 0), new CVec(1, 1) },\n+\t\t\tnew[] { new CVec(-1, -1), new CVec(-1, 0), new CVec(-1, 1), new CVec(0, 1), new CVec(1, 1) },\n+\t\t\tnew[] { new CVec(-1, 1), new CVec(0, 1), new CVec(1, 1) },\n+\t\t\tnew[] { new CVec(1, -1), new CVec(1, 0), new CVec(-1, 1), new CVec(0, 1), new CVec(1, 1) },\n \t\t};\n \n-\t\tstatic CVec[] GetNeighbors(CPos p, CPos prev)\n+\t\tstatic IEnumerable<CVec> GetNeighbors(CPos p, CPos prev)","path":"OpenRA.Mods.Common/Traits/World/PathSearch.cs","position":156,"original_position":156,"commit_id":"05a574cfb269a39a23484c9d29bb7cb7c017af7a","original_commit_id":"05a574cfb269a39a23484c9d29bb7cb7c017af7a","user":{"login":"RoosterDragon","id":3399086,"avatar_url":"https://avatars.githubusercontent.com/u/3399086?v=3","gravatar_id":"","url":"https://api.github.com/users/RoosterDragon","html_url":"https://github.com/RoosterDragon","followers_url":"https://api.github.com/users/RoosterDragon/followers","following_url":"https://api.github.com/users/RoosterDragon/following{/other_user}","gists_url":"https://api.github.com/users/RoosterDragon/gists{/gist_id}","starred_url":"https://api.github.com/users/RoosterDragon/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/RoosterDragon/subscriptions","organizations_url":"https://api.github.com/users/RoosterDragon/orgs","repos_url":"https://api.github.com/users/RoosterDragon/repos","events_url":"https://api.github.com/users/RoosterDragon/events{/privacy}","received_events_url":"https://api.github.com/users/RoosterDragon/received_events","type":"User","site_admin":false},"body":"Since this is private, it would be safe enough to keep the return type as an array. This avoids the performance penalty of enumerating via the IEnumarable interface.","created_at":"2015-02-02T20:59:46Z","updated_at":"2015-02-02T20:59:46Z","html_url":"https://github.com/OpenRA/OpenRA/pull/7358#discussion_r23957968","pull_request_url":"https://api.github.com/repos/OpenRA/OpenRA/pulls/7358","_links":{"self":{"href":"https://api.github.com/repos/OpenRA/OpenRA/pulls/comments/23957968"},"html":{"href":"https://github.com/OpenRA/OpenRA/pull/7358#discussion_r23957968"},"pull_request":{"href":"https://api.github.com/repos/OpenRA/OpenRA/pulls/7358"}}},"pull_request":{"url":"https://api.github.com/repos/OpenRA/OpenRA/pulls/7358","id":27577993,"html_url":"https://github.com/OpenRA/OpenRA/pull/7358","diff_url":"https://github.com/OpenRA/OpenRA/pull/7358.diff","patch_url":"https://github.com/OpenRA/OpenRA/pull/7358.patch","issue_url":"https://api.github.com/repos/OpenRA/OpenRA/issues/7358","number":7358,"state":"open","locked":false,"title":"Refactored the Pathfinder trait and algorithm to fit better the SOLID principles","user":{"login":"Rydra","id":5826108,"avatar_url":"https://avatars.githubusercontent.com/u/5826108?v=3","gravatar_id":"","url":"https://api.github.com/users/Rydra","html_url":"https://github.com/Rydra","followers_url":"https://api.github.com/users/Rydra/followers","following_url":"https://api.github.com/users/Rydra/following{/other_user}","gists_url":"https://api.github.com/users/Rydra/gists{/gist_id}","starred_url":"https://api.github.com/users/Rydra/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Rydra/subscriptions","organizations_url":"https://api.github.com/users/Rydra/orgs","repos_url":"https://api.github.com/users/Rydra/repos","events_url":"https://api.github.com/users/Rydra/events{/privacy}","received_events_url":"https://api.github.com/users/Rydra/received_events","type":"User","site_admin":false},"body":"The pathfinder algorithm was hard to understand and the PathFinder class violated heavily the Single Responsibility principle and the Open-Closed principle. So I refactored it using the Decorator pattern design so classes can focus on a single operation (caching, calculations, logging (on the way)) while being easily and isolately expandable instead of having one bloated and hard-to-reuse-and-extent class.\r\n\r\nI also modified the cachedpaths variable to use a Dictionary instead of a list, as order is not important in this case and insertions, deletions and lookups on a dictionary have cost O(1).\r\n\r\nThe Expand operation in PathSearch also was hard to understand, follow and unit test, so I've reorganized the responsibilities and extracted methods in an intent to make things easier to understand.","created_at":"2015-01-18T22:13:40Z","updated_at":"2015-02-02T20:59:46Z","closed_at":null,"merged_at":null,"merge_commit_sha":"988c7161431da4f178a14346e7c88a65e3671810","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/OpenRA/OpenRA/pulls/7358/commits","review_comments_url":"https://api.github.com/repos/OpenRA/OpenRA/pulls/7358/comments","review_comment_url":"https://api.github.com/repos/OpenRA/OpenRA/pulls/comments/{number}","comments_url":"https://api.github.com/repos/OpenRA/OpenRA/issues/7358/comments","statuses_url":"https://api.github.com/repos/OpenRA/OpenRA/statuses/05a574cfb269a39a23484c9d29bb7cb7c017af7a","head":{"label":"Rydra:upstream/pathfinder","ref":"upstream/pathfinder","sha":"05a574cfb269a39a23484c9d29bb7cb7c017af7a","user":{"login":"Rydra","id":5826108,"avatar_url":"https://avatars.githubusercontent.com/u/5826108?v=3","gravatar_id":"","url":"https://api.github.com/users/Rydra","html_url":"https://github.com/Rydra","followers_url":"https://api.github.com/users/Rydra/followers","following_url":"https://api.github.com/users/Rydra/following{/other_user}","gists_url":"https://api.github.com/users/Rydra/gists{/gist_id}","starred_url":"https://api.github.com/users/Rydra/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Rydra/subscriptions","organizations_url":"https://api.github.com/users/Rydra/orgs","repos_url":"https://api.github.com/users/Rydra/repos","events_url":"https://api.github.com/users/Rydra/events{/privacy}","received_events_url":"https://api.github.com/users/Rydra/received_events","type":"User","site_admin":false},"repo":{"id":19907485,"name":"OpenRA","full_name":"Rydra/OpenRA","owner":{"login":"Rydra","id":5826108,"avatar_url":"https://avatars.githubusercontent.com/u/5826108?v=3","gravatar_id":"","url":"https://api.github.com/users/Rydra","html_url":"https://github.com/Rydra","followers_url":"https://api.github.com/users/Rydra/followers","following_url":"https://api.github.com/users/Rydra/following{/other_user}","gists_url":"https://api.github.com/users/Rydra/gists{/gist_id}","starred_url":"https://api.github.com/users/Rydra/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Rydra/subscriptions","organizations_url":"https://api.github.com/users/Rydra/orgs","repos_url":"https://api.github.com/users/Rydra/repos","events_url":"https://api.github.com/users/Rydra/events{/privacy}","received_events_url":"https://api.github.com/users/Rydra/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/Rydra/OpenRA","description":"An open-source implementation of the Red Alert engine using .NET/mono and OpenGL. Runs on Windows, Linux and Mac OS X.","fork":true,"url":"https://api.github.com/repos/Rydra/OpenRA","forks_url":"https://api.github.com/repos/Rydra/OpenRA/forks","keys_url":"https://api.github.com/repos/Rydra/OpenRA/keys{/key_id}","collaborators_url":"https://api.github.com/repos/Rydra/OpenRA/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/Rydra/OpenRA/teams","hooks_url":"https://api.github.com/repos/Rydra/OpenRA/hooks","issue_events_url":"https://api.github.com/repos/Rydra/OpenRA/issues/events{/number}","events_url":"https://api.github.com/repos/Rydra/OpenRA/events","assignees_url":"https://api.github.com/repos/Rydra/OpenRA/assignees{/user}","branches_url":"https://api.github.com/repos/Rydra/OpenRA/branches{/branch}","tags_url":"https://api.github.com/repos/Rydra/OpenRA/tags","blobs_url":"https://api.github.com/repos/Rydra/OpenRA/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/Rydra/OpenRA/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/Rydra/OpenRA/git/refs{/sha}","trees_url":"https://api.github.com/repos/Rydra/OpenRA/git/trees{/sha}","statuses_url":"https://api.github.com/repos/Rydra/OpenRA/statuses/{sha}","languages_url":"https://api.github.com/repos/Rydra/OpenRA/languages","stargazers_url":"https://api.github.com/repos/Rydra/OpenRA/stargazers","contributors_url":"https://api.github.com/repos/Rydra/OpenRA/contributors","subscribers_url":"https://api.github.com/repos/Rydra/OpenRA/subscribers","subscription_url":"https://api.github.com/repos/Rydra/OpenRA/subscription","commits_url":"https://api.github.com/repos/Rydra/OpenRA/commits{/sha}","git_commits_url":"https://api.github.com/repos/Rydra/OpenRA/git/commits{/sha}","comments_url":"https://api.github.com/repos/Rydra/OpenRA/comments{/number}","issue_comment_url":"https://api.github.com/repos/Rydra/OpenRA/issues/comments/{number}","contents_url":"https://api.github.com/repos/Rydra/OpenRA/contents/{+path}","compare_url":"https://api.github.com/repos/Rydra/OpenRA/compare/{base}...{head}","merges_url":"https://api.github.com/repos/Rydra/OpenRA/merges","archive_url":"https://api.github.com/repos/Rydra/OpenRA/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/Rydra/OpenRA/downloads","issues_url":"https://api.github.com/repos/Rydra/OpenRA/issues{/number}","pulls_url":"https://api.github.com/repos/Rydra/OpenRA/pulls{/number}","milestones_url":"https://api.github.com/repos/Rydra/OpenRA/milestones{/number}","notifications_url":"https://api.github.com/repos/Rydra/OpenRA/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/Rydra/OpenRA/labels{/name}","releases_url":"https://api.github.com/repos/Rydra/OpenRA/releases{/id}","created_at":"2014-05-18T09:46:30Z","updated_at":"2015-01-27T16:14:38Z","pushed_at":"2015-02-02T19:11:15Z","git_url":"git://github.com/Rydra/OpenRA.git","ssh_url":"git@github.com:Rydra/OpenRA.git","clone_url":"https://github.com/Rydra/OpenRA.git","svn_url":"https://github.com/Rydra/OpenRA","homepage":"http://open-ra.org","size":80119,"stargazers_count":1,"watchers_count":1,"language":"C#","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":1,"default_branch":"bleed"}},"base":{"label":"OpenRA:bleed","ref":"bleed","sha":"b68918f5d5f0d71a09e0afd3561e9b21eb2e0617","user":{"login":"OpenRA","id":409046,"avatar_url":"https://avatars.githubusercontent.com/u/409046?v=3","gravatar_id":"","url":"https://api.github.com/users/OpenRA","html_url":"https://github.com/OpenRA","followers_url":"https://api.github.com/users/OpenRA/followers","following_url":"https://api.github.com/users/OpenRA/following{/other_user}","gists_url":"https://api.github.com/users/OpenRA/gists{/gist_id}","starred_url":"https://api.github.com/users/OpenRA/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/OpenRA/subscriptions","organizations_url":"https://api.github.com/users/OpenRA/orgs","repos_url":"https://api.github.com/users/OpenRA/repos","events_url":"https://api.github.com/users/OpenRA/events{/privacy}","received_events_url":"https://api.github.com/users/OpenRA/received_events","type":"Organization","site_admin":false},"repo":{"id":959908,"name":"OpenRA","full_name":"OpenRA/OpenRA","owner":{"login":"OpenRA","id":409046,"avatar_url":"https://avatars.githubusercontent.com/u/409046?v=3","gravatar_id":"","url":"https://api.github.com/users/OpenRA","html_url":"https://github.com/OpenRA","followers_url":"https://api.github.com/users/OpenRA/followers","following_url":"https://api.github.com/users/OpenRA/following{/other_user}","gists_url":"https://api.github.com/users/OpenRA/gists{/gist_id}","starred_url":"https://api.github.com/users/OpenRA/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/OpenRA/subscriptions","organizations_url":"https://api.github.com/users/OpenRA/orgs","repos_url":"https://api.github.com/users/OpenRA/repos","events_url":"https://api.github.com/users/OpenRA/events{/privacy}","received_events_url":"https://api.github.com/users/OpenRA/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/OpenRA/OpenRA","description":"An open-source implementation of the Command & Conquer: Red Alert engine using .NET/Mono and OpenGL. Runs on Windows, Linux and Mac OS X.","fork":false,"url":"https://api.github.com/repos/OpenRA/OpenRA","forks_url":"https://api.github.com/repos/OpenRA/OpenRA/forks","keys_url":"https://api.github.com/repos/OpenRA/OpenRA/keys{/key_id}","collaborators_url":"https://api.github.com/repos/OpenRA/OpenRA/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/OpenRA/OpenRA/teams","hooks_url":"https://api.github.com/repos/OpenRA/OpenRA/hooks","issue_events_url":"https://api.github.com/repos/OpenRA/OpenRA/issues/events{/number}","events_url":"https://api.github.com/repos/OpenRA/OpenRA/events","assignees_url":"https://api.github.com/repos/OpenRA/OpenRA/assignees{/user}","branches_url":"https://api.github.com/repos/OpenRA/OpenRA/branches{/branch}","tags_url":"https://api.github.com/repos/OpenRA/OpenRA/tags","blobs_url":"https://api.github.com/repos/OpenRA/OpenRA/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/OpenRA/OpenRA/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/OpenRA/OpenRA/git/refs{/sha}","trees_url":"https://api.github.com/repos/OpenRA/OpenRA/git/trees{/sha}","statuses_url":"https://api.github.com/repos/OpenRA/OpenRA/statuses/{sha}","languages_url":"https://api.github.com/repos/OpenRA/OpenRA/languages","stargazers_url":"https://api.github.com/repos/OpenRA/OpenRA/stargazers","contributors_url":"https://api.github.com/repos/OpenRA/OpenRA/contributors","subscribers_url":"https://api.github.com/repos/OpenRA/OpenRA/subscribers","subscription_url":"https://api.github.com/repos/OpenRA/OpenRA/subscription","commits_url":"https://api.github.com/repos/OpenRA/OpenRA/commits{/sha}","git_commits_url":"https://api.github.com/repos/OpenRA/OpenRA/git/commits{/sha}","comments_url":"https://api.github.com/repos/OpenRA/OpenRA/comments{/number}","issue_comment_url":"https://api.github.com/repos/OpenRA/OpenRA/issues/comments/{number}","contents_url":"https://api.github.com/repos/OpenRA/OpenRA/contents/{+path}","compare_url":"https://api.github.com/repos/OpenRA/OpenRA/compare/{base}...{head}","merges_url":"https://api.github.com/repos/OpenRA/OpenRA/merges","archive_url":"https://api.github.com/repos/OpenRA/OpenRA/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/OpenRA/OpenRA/downloads","issues_url":"https://api.github.com/repos/OpenRA/OpenRA/issues{/number}","pulls_url":"https://api.github.com/repos/OpenRA/OpenRA/pulls{/number}","milestones_url":"https://api.github.com/repos/OpenRA/OpenRA/milestones{/number}","notifications_url":"https://api.github.com/repos/OpenRA/OpenRA/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/OpenRA/OpenRA/labels{/name}","releases_url":"https://api.github.com/repos/OpenRA/OpenRA/releases{/id}","created_at":"2010-10-04T07:38:05Z","updated_at":"2015-02-02T17:38:31Z","pushed_at":"2015-02-02T16:06:54Z","git_url":"git://github.com/OpenRA/OpenRA.git","ssh_url":"git@github.com:OpenRA/OpenRA.git","clone_url":"https://github.com/OpenRA/OpenRA.git","svn_url":"https://github.com/OpenRA/OpenRA","homepage":"http://www.openra.net","size":154067,"stargazers_count":1682,"watchers_count":1682,"language":"C#","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":583,"mirror_url":null,"open_issues_count":692,"forks":583,"open_issues":692,"watchers":1682,"default_branch":"bleed"}},"_links":{"self":{"href":"https://api.github.com/repos/OpenRA/OpenRA/pulls/7358"},"html":{"href":"https://github.com/OpenRA/OpenRA/pull/7358"},"issue":{"href":"https://api.github.com/repos/OpenRA/OpenRA/issues/7358"},"comments":{"href":"https://api.github.com/repos/OpenRA/OpenRA/issues/7358/comments"},"review_comments":{"href":"https://api.github.com/repos/OpenRA/OpenRA/pulls/7358/comments"},"review_comment":{"href":"https://api.github.com/repos/OpenRA/OpenRA/pulls/comments/{number}"},"commits":{"href":"https://api.github.com/repos/OpenRA/OpenRA/pulls/7358/commits"},"statuses":{"href":"https://api.github.com/repos/OpenRA/OpenRA/statuses/05a574cfb269a39a23484c9d29bb7cb7c017af7a"}}}}