{"action":"created","issue":{"url":"https://api.github.com/repos/shardingjdbc/sharding-jdbc/issues/718","repository_url":"https://api.github.com/repos/shardingjdbc/sharding-jdbc","labels_url":"https://api.github.com/repos/shardingjdbc/sharding-jdbc/issues/718/labels{/name}","comments_url":"https://api.github.com/repos/shardingjdbc/sharding-jdbc/issues/718/comments","events_url":"https://api.github.com/repos/shardingjdbc/sharding-jdbc/issues/718/events","html_url":"https://github.com/shardingjdbc/sharding-jdbc/pull/718","id":313574370,"number":718,"title":"Rewrite getShardingValues and call getShardingValues move to ParsingSQLRouter class","user":{"login":"ma-xiao-guang-64","id":6637227,"avatar_url":"https://avatars1.githubusercontent.com/u/6637227?v=4","gravatar_id":"","url":"https://api.github.com/users/ma-xiao-guang-64","html_url":"https://github.com/ma-xiao-guang-64","followers_url":"https://api.github.com/users/ma-xiao-guang-64/followers","following_url":"https://api.github.com/users/ma-xiao-guang-64/following{/other_user}","gists_url":"https://api.github.com/users/ma-xiao-guang-64/gists{/gist_id}","starred_url":"https://api.github.com/users/ma-xiao-guang-64/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ma-xiao-guang-64/subscriptions","organizations_url":"https://api.github.com/users/ma-xiao-guang-64/orgs","repos_url":"https://api.github.com/users/ma-xiao-guang-64/repos","events_url":"https://api.github.com/users/ma-xiao-guang-64/events{/privacy}","received_events_url":"https://api.github.com/users/ma-xiao-guang-64/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2018-04-12T04:39:28Z","updated_at":"2018-04-12T16:10:31Z","closed_at":"2018-04-12T14:28:02Z","author_association":"COLLABORATOR","pull_request":{"url":"https://api.github.com/repos/shardingjdbc/sharding-jdbc/pulls/718","html_url":"https://github.com/shardingjdbc/sharding-jdbc/pull/718","diff_url":"https://github.com/shardingjdbc/sharding-jdbc/pull/718.diff","patch_url":"https://github.com/shardingjdbc/sharding-jdbc/pull/718.patch"},"body":"1、add function getShardingValues to Conditions class(Conditions增加getShardingValues方法)\r\n2、add some wrapper class with ShardingValue(增加关于ShardingValue的一些包装类)\r\n3、sharding jdbc use unicase route when where caluse in sql is always false(当sql条件为永假时使用Unicase route)"},"comment":{"url":"https://api.github.com/repos/shardingjdbc/sharding-jdbc/issues/comments/380860024","html_url":"https://github.com/shardingjdbc/sharding-jdbc/pull/718#issuecomment-380860024","issue_url":"https://api.github.com/repos/shardingjdbc/sharding-jdbc/issues/718","id":380860024,"user":{"login":"terrymanu","id":5516298,"avatar_url":"https://avatars1.githubusercontent.com/u/5516298?v=4","gravatar_id":"","url":"https://api.github.com/users/terrymanu","html_url":"https://github.com/terrymanu","followers_url":"https://api.github.com/users/terrymanu/followers","following_url":"https://api.github.com/users/terrymanu/following{/other_user}","gists_url":"https://api.github.com/users/terrymanu/gists{/gist_id}","starred_url":"https://api.github.com/users/terrymanu/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/terrymanu/subscriptions","organizations_url":"https://api.github.com/users/terrymanu/orgs","repos_url":"https://api.github.com/users/terrymanu/repos","events_url":"https://api.github.com/users/terrymanu/events{/privacy}","received_events_url":"https://api.github.com/users/terrymanu/received_events","type":"User","site_admin":false},"created_at":"2018-04-12T16:10:31Z","updated_at":"2018-04-12T16:10:31Z","author_association":"OWNER","body":"Code review suggestion: \r\n\r\n1. Condition 111行，getValues方法，为什么改为public？\r\n2. Conditions 61行和72行的find方法，只有测试在使用，是否可以删除？\r\n3. ParsingSQLRouter 193行getShardingConditions方法的创建result的逻辑有点混乱，是否可以在一开始就直接new ShardingConditions？而不是让其有可能返回null。如无特殊理由，sj的代码中不允许返回null，不应用null做判断，如：ParsingSQLRouter的139行\r\n4. ShardingConditions 35行，generatedKey无处调用，是做什么的？\r\n5. ShardingConditions 38行，不应为shardingConditions提供get方法，因为所有的访问都已经被封装。\r\n6. ShardingConditions 38行，调用get方法，不能使用LinkedList\r\n7. ParsingSQLRouter 212行，不用直接调用andCondition.getConditions().isEmpty()，而是使用andCondition.isEmpty()，由此推断，andCondition.getConditions()是否不应提供？\r\n8. ParsingSQLRouter 216，265行，不要在代码中使用Map.Entry，直接import Map.Entry，在代码中使用Entry即可\r\n9. ParsingSQLRouter 224行，不要return，考虑重构为别的方式\r\n10. ParsingSQLRouter 252，279，293，303行 checkstyle未通过，参数需要加final\r\n11. ParsingSQLRouter 253行，new LinkedHashMap<>()在已知size的情况下未设置size大小，应该为new LinkedHashMap<>(andCondition.getConditions().size(), 1)\r\n12. ParsingSQLRouter 255行，不要使用null == result.get(each.getColumn())进行map判断，代替为result.containsKey(each.getColumn())\r\n13. ParsingSQLRouter 210行，getShardingCondition等大量方法需要考虑新建对象，提取到ParsingSQLRouter之外\r\n14. ParsingSQLRouter 288，311行，考虑不要使用null，考虑使用ForeverFalseShardingCondition\r\n15. ParsingSQLRouter 221行，为什么使用Objects.equal，考虑直接使用ShardingOperator.EQUAL == each.getOperator()\r\n16. StandardRoutingEngine 56行，删除参数parameters，这个构造器没用了，可以直接删了，用@RequiredArgsConstructor即可\r\n17. StandardRoutingEngine 169行 checkstyle未通过，参数需要加final\r\n18. StandardRoutingEngine 109行 getShardingValues，整个方法层级太多，应合理使用卫语句，如：111行改为if (shardingConditions.isEmpty()) {\r\nreturn；\r\n}\r\n19. StandardRoutingEngine 115行，不要使用Objects.equal,logicTableName不可能为null\r\n20. ShardingCondition封装层级不一致，可以考虑要么完全放开，通过getShardingValues().add添加，要么全部封装，不提供getShardingValues()方法\r\n21. ShardingConditions 77行，size只被内部用，可以直接删掉，内部用shardingConditions.size()即可\r\n22. UnicastRoutingEngine 42行，sqlStatement没用可否删掉？\r\n23. UnicastRoutingEngine 53行，等于的判断都是常量在前，应该为1 == logicTables.size()\r\n24. UnicastRoutingEngine新加的59行之后的逻辑，不应放在UnicastRoutingEngine中。因为UnicastRoutingEngine的意思是单播，也就是选中其中的一个节点路由。else的新增部分已经不是单播。因此这里的代码必须剥离，不能放在这个类中，需要考虑重新设计\r\n25. ShardingRule中新增的方法findDataNodeByDataSourceAndLogicTable，需要考虑与本来存在的findDataNodeByLogicTable方法复用。比如，重构findDataNodeByLogicTable，让其调用findDataNodeByDataSourceAndLogicTable\r\n26. 将ShardingRule中的findDataNodeByDataSourceAndLogicTable与findDataNodeByLogicTable都改名为findDataNode的重载方法\r\n\r\n\r\n最后，在你修改过的类后面加上自己的名字吧，比如，原来的注释：\r\n\r\n```\r\n/**\r\n * Sharding conditions.\r\n *\r\n * @author zhangliang\r\n */\r\n\r\n 改为：\r\n\r\n/**\r\n * Sharding conditions.\r\n *\r\n * @author zhangliang\r\n * @author your name\r\n */\r\n```\r\n\r\n"}}
{"actor":{"display_login":"terrymanu"}}