{"action":"created","comment":{"url":"https://api.github.com/repos/graphhopper/graphhopper/pulls/comments/147904313","pull_request_review_id":73064617,"id":147904313,"diff_hunk":"@@ -11,4 +11,6 @@\n     DecimalEncodedValue getDecimalEncodedValue(String key);\n \n     StringEncodedValue getStringEncodedValue(String key);\n+\n+    boolean containsEncodedValue(String key);","path":"core/src/main/java/com/graphhopper/routing/profiles/EncodedValueLookup.java","position":null,"original_position":5,"commit_id":"4232692e9d28355aba428128597cb4568126be2d","original_commit_id":"74eda142a1adc0943b1c8def058553e699682d85","user":{"login":"boldtrn","id":1553525,"avatar_url":"https://avatars3.githubusercontent.com/u/1553525?v=4","gravatar_id":"","url":"https://api.github.com/users/boldtrn","html_url":"https://github.com/boldtrn","followers_url":"https://api.github.com/users/boldtrn/followers","following_url":"https://api.github.com/users/boldtrn/following{/other_user}","gists_url":"https://api.github.com/users/boldtrn/gists{/gist_id}","starred_url":"https://api.github.com/users/boldtrn/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/boldtrn/subscriptions","organizations_url":"https://api.github.com/users/boldtrn/orgs","repos_url":"https://api.github.com/users/boldtrn/repos","events_url":"https://api.github.com/users/boldtrn/events{/privacy}","received_events_url":"https://api.github.com/users/boldtrn/received_events","type":"User","site_admin":false},"body":"> The implementation in the AbstractFlagEncoder could be done with the encodedValueLookup\r\n\r\nThe issue with this is that we would return the values that are stored in the EncodingManager, but maybe not with/because of this encoder. I am not sure if this is important. For example:\r\n\r\n```\r\nEncodingManager em = new EncodingManager.Builder().addGlobalEncodedValues().addAllFlagEncoders(\"car,bike,foot\").build();\r\nCarFlagEncoder encoder = (CarFlagEncoder) em.getEncoder(\"car\");\r\n\r\nencoder.containsEncodedValue(TagParserFactory.BIKE_ACCESS) // => is true\r\n```\r\n\r\nFor me this seems a bit weird. But maybe it seems weird to me because I am still thinking too much about the \"old\" system? ","created_at":"2017-10-31T06:31:51Z","updated_at":"2017-10-31T06:31:51Z","html_url":"https://github.com/graphhopper/graphhopper/pull/1112#discussion_r147904313","pull_request_url":"https://api.github.com/repos/graphhopper/graphhopper/pulls/1112","author_association":"MEMBER","_links":{"self":{"href":"https://api.github.com/repos/graphhopper/graphhopper/pulls/comments/147904313"},"html":{"href":"https://github.com/graphhopper/graphhopper/pull/1112#discussion_r147904313"},"pull_request":{"href":"https://api.github.com/repos/graphhopper/graphhopper/pulls/1112"}},"in_reply_to_id":146755341},"pull_request":{"url":"https://api.github.com/repos/graphhopper/graphhopper/pulls/1112","id":130363978,"html_url":"https://github.com/graphhopper/graphhopper/pull/1112","diff_url":"https://github.com/graphhopper/graphhopper/pull/1112.diff","patch_url":"https://github.com/graphhopper/graphhopper/pull/1112.patch","issue_url":"https://api.github.com/repos/graphhopper/graphhopper/issues/1112","number":1112,"state":"open","locked":false,"title":"(WIP) EncodingManager refactoring","user":{"login":"karussell","id":129644,"avatar_url":"https://avatars0.githubusercontent.com/u/129644?v=4","gravatar_id":"","url":"https://api.github.com/users/karussell","html_url":"https://github.com/karussell","followers_url":"https://api.github.com/users/karussell/followers","following_url":"https://api.github.com/users/karussell/following{/other_user}","gists_url":"https://api.github.com/users/karussell/gists{/gist_id}","starred_url":"https://api.github.com/users/karussell/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/karussell/subscriptions","organizations_url":"https://api.github.com/users/karussell/orgs","repos_url":"https://api.github.com/users/karussell/repos","events_url":"https://api.github.com/users/karussell/events{/privacy}","received_events_url":"https://api.github.com/users/karussell/received_events","type":"User","site_admin":false},"body":"This is a first sketch of a new approach to improve the flexibility for the edge property storage and it fixes #472, #728, #978 and probably some more issues. \r\n\r\nPlease do not look at the ugliness yet ;), just look at the [FastestCarWeighting](https://github.com/graphhopper/graphhopper/blob/8dd658c5ae48e8a690a90ed6c9135d4ae7d7479a/core/src/main/java/com/graphhopper/routing/weighting/FastestCarWeighting.java) and how we fetch the values in the constructor that are necessary  in the calcWeight method, really easy and understandable:\r\n\r\n```java\r\ndouble speed = edgeState.get(averageSpeed);\r\n```\r\n\r\nThese values are composed upfront and are responsible just for a value (and optionally a reverse value), have a look into [OSMReaderTest.testEncodedValueBasedEncodingManager](https://github.com/graphhopper/graphhopper/pull/1112/files#diff-100e0c53430e5948905a11dcb0821360R812) for a complete test parsing an OSM file.\r\n\r\nSo all in all I was able to remove the necessity of the FlagEncoder classes. So just EncodingManager and a list of EncodedValue is necessary. And the best: this already works within the existing system and the new EncodingManager is compatible with the old.\r\n\r\nI've created a EncodedValueFactory to create some default EncodedValues like maxspeed, surface, 'car access' etc so that useful Weightings can be created.\r\n\r\nThe main changes in this PR:\r\n\r\n 1. `long flags` was replaced with a IntsRef that is not limited to 64bits (later on we could use this for all edge properties like distance and pointers, also there is a minor advantage that we could point directly into the storage backing array instead of copying but this is for a later PR)\r\n 2. new directed EncodedValue that are able to store forward and backward values, no need for expensive reverseFlags method, much simpler usage in Weighting implementations, no need for flag encoders but still backward compatible to them\r\n 3. JSON (de)serialization of storage properties\r\n\r\nFurther work items:\r\n\r\n - rename edge.getData to getValues?\r\n - use new EncodedValue classes in old FlagEncoder classes: DataFlagEncoder, CarFlagEncoder, ...\r\n - merge master into this branch\r\n - what to do with the unfavoredEdge used only in QueryGraph -> do we need a 0-bit EncodedValue used just as a 'marker'?\r\n - use two EncodedValues for shortcuts two (weight and access) in CHGraphImpl -> advantage would be that we would be able to store weight with higher precision\r\n - we could introduce a \"reverse\" EdgeExplorer or `iter.setBaseNode(node, reverse=true)` and then avoid the reverse parameter in Weighting.calcWeight as well as the if clauses to fetch e.g. the forward or the reverse speed. I.e. one edge is directed API-wise and undirected storage-wise. We should do this undirected vs. directed stuff in a later step, see also https://github.com/graphhopper/graphhopper/pull/1112#issuecomment-322522398\r\n - what to do if a reverse read-out of a oneway edge: exception vs. silently ok? how would we do the exception - via a new setting \"read of default value results in exception\" or somehow associating the `access`-EncodedValue with e.g. the `max_speed`-EncValue?\r\n - version of every EncodedValue to make loading graph more secure?\r\n - avoid Weighting.getFlagEncoder\r\n - replace AbstractFlagEncoder and FlagEncoder with something less ugly, separate turn and node flags (Profile == collection of EncodedValue or ComposedEncodedValues)\r\n - maybe rename all XY08 into XYOld or XY09 instead? Add a deprecation notice and removal with 1.x\r\n - remove all the TODOs and ugly hacks\r\n - copyProperties can be done via array copy and setting just the node indices afterwards\r\n - define TagParser order\r\n - allow cross-integer space for EncodedValue otherwise we can get bad constellations e.g. if EncodedValue has two directions and requires e.g. 17 bits\r\n - move TagParserFactory constants into Parameters?\r\n - ~~merge new EncodingManager with old~~\r\n - ~~try a real world example and parse OSM with a car profile and query via the new weighting mechanism~~\r\n - ~~use IntRefs instead of int flags~~\r\n - ~~a directed DoubleProperty and BooleanProperty class that holds two values (one value for a direction) that can be easily 'reversed' -> ComposedEncodedValue holds multiple encoded values? support get and getReversed directly for every encodedValue?~~\r\n - OSMReader is currently doing duplicate work to calculate bits of one OSM way, instead it should calculate the bits once and just array copy to the other edges using edge.get&setData?\r\n - ~~should we introduce a PropertyFilter so that a maxspeed is not parsed and calculated if the highway tag is not accepted ('rail')?~~\r\n - PathExtract should be more separated (call extract explicitly after calcPath and potentially rename calcPath to calcShortestPathTree)\r\n - ~~move out EncodedValue.parse into something else to make support of non-OSM easier~~\r\n - InstructionAnnotation getAnnotation -> remove this or replace by a ComposedEncodedValue that can warn about private, tool, ferry or ford access?\r\n - we could store the distance as an EncodedValue\r\n - But in general we need length dependent mechanism somehow\r\n - reimplement the CarFlagEncoder as a ComposedEncodedValue?\r\n - separate TurnWeighting handling -> how to do  storage of OSM-node and OSM-relation information?","created_at":"2017-07-13T12:58:38Z","updated_at":"2017-10-31T06:31:51Z","closed_at":null,"merged_at":null,"merge_commit_sha":"cf98d8d1a9daa9a059f40fe87792d5e82a9ff0f8","assignee":null,"assignees":[],"requested_reviewers":[],"milestone":null,"commits_url":"https://api.github.com/repos/graphhopper/graphhopper/pulls/1112/commits","review_comments_url":"https://api.github.com/repos/graphhopper/graphhopper/pulls/1112/comments","review_comment_url":"https://api.github.com/repos/graphhopper/graphhopper/pulls/comments{/number}","comments_url":"https://api.github.com/repos/graphhopper/graphhopper/issues/1112/comments","statuses_url":"https://api.github.com/repos/graphhopper/graphhopper/statuses/4232692e9d28355aba428128597cb4568126be2d","head":{"label":"graphhopper:encoder_refactoring","ref":"encoder_refactoring","sha":"4232692e9d28355aba428128597cb4568126be2d","user":{"login":"graphhopper","id":2264498,"avatar_url":"https://avatars1.githubusercontent.com/u/2264498?v=4","gravatar_id":"","url":"https://api.github.com/users/graphhopper","html_url":"https://github.com/graphhopper","followers_url":"https://api.github.com/users/graphhopper/followers","following_url":"https://api.github.com/users/graphhopper/following{/other_user}","gists_url":"https://api.github.com/users/graphhopper/gists{/gist_id}","starred_url":"https://api.github.com/users/graphhopper/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/graphhopper/subscriptions","organizations_url":"https://api.github.com/users/graphhopper/orgs","repos_url":"https://api.github.com/users/graphhopper/repos","events_url":"https://api.github.com/users/graphhopper/events{/privacy}","received_events_url":"https://api.github.com/users/graphhopper/received_events","type":"Organization","site_admin":false},"repo":{"id":3480666,"name":"graphhopper","full_name":"graphhopper/graphhopper","owner":{"login":"graphhopper","id":2264498,"avatar_url":"https://avatars1.githubusercontent.com/u/2264498?v=4","gravatar_id":"","url":"https://api.github.com/users/graphhopper","html_url":"https://github.com/graphhopper","followers_url":"https://api.github.com/users/graphhopper/followers","following_url":"https://api.github.com/users/graphhopper/following{/other_user}","gists_url":"https://api.github.com/users/graphhopper/gists{/gist_id}","starred_url":"https://api.github.com/users/graphhopper/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/graphhopper/subscriptions","organizations_url":"https://api.github.com/users/graphhopper/orgs","repos_url":"https://api.github.com/users/graphhopper/repos","events_url":"https://api.github.com/users/graphhopper/events{/privacy}","received_events_url":"https://api.github.com/users/graphhopper/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/graphhopper/graphhopper","description":"An open source route planning library and server using OpenStreetMap.","fork":false,"url":"https://api.github.com/repos/graphhopper/graphhopper","forks_url":"https://api.github.com/repos/graphhopper/graphhopper/forks","keys_url":"https://api.github.com/repos/graphhopper/graphhopper/keys{/key_id}","collaborators_url":"https://api.github.com/repos/graphhopper/graphhopper/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/graphhopper/graphhopper/teams","hooks_url":"https://api.github.com/repos/graphhopper/graphhopper/hooks","issue_events_url":"https://api.github.com/repos/graphhopper/graphhopper/issues/events{/number}","events_url":"https://api.github.com/repos/graphhopper/graphhopper/events","assignees_url":"https://api.github.com/repos/graphhopper/graphhopper/assignees{/user}","branches_url":"https://api.github.com/repos/graphhopper/graphhopper/branches{/branch}","tags_url":"https://api.github.com/repos/graphhopper/graphhopper/tags","blobs_url":"https://api.github.com/repos/graphhopper/graphhopper/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/graphhopper/graphhopper/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/graphhopper/graphhopper/git/refs{/sha}","trees_url":"https://api.github.com/repos/graphhopper/graphhopper/git/trees{/sha}","statuses_url":"https://api.github.com/repos/graphhopper/graphhopper/statuses/{sha}","languages_url":"https://api.github.com/repos/graphhopper/graphhopper/languages","stargazers_url":"https://api.github.com/repos/graphhopper/graphhopper/stargazers","contributors_url":"https://api.github.com/repos/graphhopper/graphhopper/contributors","subscribers_url":"https://api.github.com/repos/graphhopper/graphhopper/subscribers","subscription_url":"https://api.github.com/repos/graphhopper/graphhopper/subscription","commits_url":"https://api.github.com/repos/graphhopper/graphhopper/commits{/sha}","git_commits_url":"https://api.github.com/repos/graphhopper/graphhopper/git/commits{/sha}","comments_url":"https://api.github.com/repos/graphhopper/graphhopper/comments{/number}","issue_comment_url":"https://api.github.com/repos/graphhopper/graphhopper/issues/comments{/number}","contents_url":"https://api.github.com/repos/graphhopper/graphhopper/contents/{+path}","compare_url":"https://api.github.com/repos/graphhopper/graphhopper/compare/{base}...{head}","merges_url":"https://api.github.com/repos/graphhopper/graphhopper/merges","archive_url":"https://api.github.com/repos/graphhopper/graphhopper/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/graphhopper/graphhopper/downloads","issues_url":"https://api.github.com/repos/graphhopper/graphhopper/issues{/number}","pulls_url":"https://api.github.com/repos/graphhopper/graphhopper/pulls{/number}","milestones_url":"https://api.github.com/repos/graphhopper/graphhopper/milestones{/number}","notifications_url":"https://api.github.com/repos/graphhopper/graphhopper/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/graphhopper/graphhopper/labels{/name}","releases_url":"https://api.github.com/repos/graphhopper/graphhopper/releases{/id}","deployments_url":"https://api.github.com/repos/graphhopper/graphhopper/deployments","created_at":"2012-02-18T19:57:38Z","updated_at":"2017-10-31T04:04:45Z","pushed_at":"2017-10-31T00:02:59Z","git_url":"git://github.com/graphhopper/graphhopper.git","ssh_url":"git@github.com:graphhopper/graphhopper.git","clone_url":"https://github.com/graphhopper/graphhopper.git","svn_url":"https://github.com/graphhopper/graphhopper","homepage":"https://www.graphhopper.com/open-source/","size":63260,"stargazers_count":1342,"watchers_count":1342,"language":"Java","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":563,"mirror_url":null,"archived":false,"open_issues_count":203,"forks":563,"open_issues":203,"watchers":1342,"default_branch":"master"}},"base":{"label":"graphhopper:master","ref":"master","sha":"c82c242096cb0c7744325fbd0394b724983f64e1","user":{"login":"graphhopper","id":2264498,"avatar_url":"https://avatars1.githubusercontent.com/u/2264498?v=4","gravatar_id":"","url":"https://api.github.com/users/graphhopper","html_url":"https://github.com/graphhopper","followers_url":"https://api.github.com/users/graphhopper/followers","following_url":"https://api.github.com/users/graphhopper/following{/other_user}","gists_url":"https://api.github.com/users/graphhopper/gists{/gist_id}","starred_url":"https://api.github.com/users/graphhopper/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/graphhopper/subscriptions","organizations_url":"https://api.github.com/users/graphhopper/orgs","repos_url":"https://api.github.com/users/graphhopper/repos","events_url":"https://api.github.com/users/graphhopper/events{/privacy}","received_events_url":"https://api.github.com/users/graphhopper/received_events","type":"Organization","site_admin":false},"repo":{"id":3480666,"name":"graphhopper","full_name":"graphhopper/graphhopper","owner":{"login":"graphhopper","id":2264498,"avatar_url":"https://avatars1.githubusercontent.com/u/2264498?v=4","gravatar_id":"","url":"https://api.github.com/users/graphhopper","html_url":"https://github.com/graphhopper","followers_url":"https://api.github.com/users/graphhopper/followers","following_url":"https://api.github.com/users/graphhopper/following{/other_user}","gists_url":"https://api.github.com/users/graphhopper/gists{/gist_id}","starred_url":"https://api.github.com/users/graphhopper/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/graphhopper/subscriptions","organizations_url":"https://api.github.com/users/graphhopper/orgs","repos_url":"https://api.github.com/users/graphhopper/repos","events_url":"https://api.github.com/users/graphhopper/events{/privacy}","received_events_url":"https://api.github.com/users/graphhopper/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/graphhopper/graphhopper","description":"An open source route planning library and server using OpenStreetMap.","fork":false,"url":"https://api.github.com/repos/graphhopper/graphhopper","forks_url":"https://api.github.com/repos/graphhopper/graphhopper/forks","keys_url":"https://api.github.com/repos/graphhopper/graphhopper/keys{/key_id}","collaborators_url":"https://api.github.com/repos/graphhopper/graphhopper/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/graphhopper/graphhopper/teams","hooks_url":"https://api.github.com/repos/graphhopper/graphhopper/hooks","issue_events_url":"https://api.github.com/repos/graphhopper/graphhopper/issues/events{/number}","events_url":"https://api.github.com/repos/graphhopper/graphhopper/events","assignees_url":"https://api.github.com/repos/graphhopper/graphhopper/assignees{/user}","branches_url":"https://api.github.com/repos/graphhopper/graphhopper/branches{/branch}","tags_url":"https://api.github.com/repos/graphhopper/graphhopper/tags","blobs_url":"https://api.github.com/repos/graphhopper/graphhopper/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/graphhopper/graphhopper/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/graphhopper/graphhopper/git/refs{/sha}","trees_url":"https://api.github.com/repos/graphhopper/graphhopper/git/trees{/sha}","statuses_url":"https://api.github.com/repos/graphhopper/graphhopper/statuses/{sha}","languages_url":"https://api.github.com/repos/graphhopper/graphhopper/languages","stargazers_url":"https://api.github.com/repos/graphhopper/graphhopper/stargazers","contributors_url":"https://api.github.com/repos/graphhopper/graphhopper/contributors","subscribers_url":"https://api.github.com/repos/graphhopper/graphhopper/subscribers","subscription_url":"https://api.github.com/repos/graphhopper/graphhopper/subscription","commits_url":"https://api.github.com/repos/graphhopper/graphhopper/commits{/sha}","git_commits_url":"https://api.github.com/repos/graphhopper/graphhopper/git/commits{/sha}","comments_url":"https://api.github.com/repos/graphhopper/graphhopper/comments{/number}","issue_comment_url":"https://api.github.com/repos/graphhopper/graphhopper/issues/comments{/number}","contents_url":"https://api.github.com/repos/graphhopper/graphhopper/contents/{+path}","compare_url":"https://api.github.com/repos/graphhopper/graphhopper/compare/{base}...{head}","merges_url":"https://api.github.com/repos/graphhopper/graphhopper/merges","archive_url":"https://api.github.com/repos/graphhopper/graphhopper/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/graphhopper/graphhopper/downloads","issues_url":"https://api.github.com/repos/graphhopper/graphhopper/issues{/number}","pulls_url":"https://api.github.com/repos/graphhopper/graphhopper/pulls{/number}","milestones_url":"https://api.github.com/repos/graphhopper/graphhopper/milestones{/number}","notifications_url":"https://api.github.com/repos/graphhopper/graphhopper/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/graphhopper/graphhopper/labels{/name}","releases_url":"https://api.github.com/repos/graphhopper/graphhopper/releases{/id}","deployments_url":"https://api.github.com/repos/graphhopper/graphhopper/deployments","created_at":"2012-02-18T19:57:38Z","updated_at":"2017-10-31T04:04:45Z","pushed_at":"2017-10-31T00:02:59Z","git_url":"git://github.com/graphhopper/graphhopper.git","ssh_url":"git@github.com:graphhopper/graphhopper.git","clone_url":"https://github.com/graphhopper/graphhopper.git","svn_url":"https://github.com/graphhopper/graphhopper","homepage":"https://www.graphhopper.com/open-source/","size":63260,"stargazers_count":1342,"watchers_count":1342,"language":"Java","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":563,"mirror_url":null,"archived":false,"open_issues_count":203,"forks":563,"open_issues":203,"watchers":1342,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/graphhopper/graphhopper/pulls/1112"},"html":{"href":"https://github.com/graphhopper/graphhopper/pull/1112"},"issue":{"href":"https://api.github.com/repos/graphhopper/graphhopper/issues/1112"},"comments":{"href":"https://api.github.com/repos/graphhopper/graphhopper/issues/1112/comments"},"review_comments":{"href":"https://api.github.com/repos/graphhopper/graphhopper/pulls/1112/comments"},"review_comment":{"href":"https://api.github.com/repos/graphhopper/graphhopper/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/graphhopper/graphhopper/pulls/1112/commits"},"statuses":{"href":"https://api.github.com/repos/graphhopper/graphhopper/statuses/4232692e9d28355aba428128597cb4568126be2d"}},"author_association":"OWNER"}}
{"actor":{"display_login":"boldtrn"}}