)]}'
{"barbicanclient/orders.py":[{"author":{"_account_id":6783,"name":"Nathan Reller","email":"nathan.s.reller@gmail.com","username":"rellerreller"},"change_message_id":"eeda5859e158c64aa61193ded255ffd08dc22c44","unresolved":false,"context_lines":[{"line_number":419,"context_line":"        if type:"},{"line_number":420,"context_line":"            order_type \u003d self._order_type_to_class_map.get(type.lower())"},{"line_number":421,"context_line":"            if order_type is not None:"},{"line_number":422,"context_line":"                return order_type(self._api, **kwargs)"},{"line_number":423,"context_line":"        else:"},{"line_number":424,"context_line":"            raise TypeError(\u0027Unknown Order type \"{0}\"\u0027.format(type))"},{"line_number":425,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9a1a9d01_704a6d85","line":422,"updated":"2015-09-25 13:12:03.000000000","message":"My personal preference on parameter checks are that they should go first and raise an error if not correct. This way you do not have a pattern like the following:\n\nif parameter is valid:\n  do business stuff\n  do business stuff\n  do business stuff\nelse:\n  raise error\n\nThe problem with this pattern is \u0027do business stuff\u0027 can be many lines long, easily 30+ lines. By the time you get to the else statement then you forgot what the corresponding if statement is.\n\nIf you change the code to do the parameter checking first then catch all bad parameters first and then do business logic. It looks like the following:\n\nif parameter is invalid:\n  raise error\ndo business stuff\ndo business stuff\ndo business stuff\n\nI think it makes it easy to see what the expected values are and no extra indentation.","commit_id":"fc0f7d13bb07145cfde3ac4a858bd37494b794ff"},{"author":{"_account_id":6783,"name":"Nathan Reller","email":"nathan.s.reller@gmail.com","username":"rellerreller"},"change_message_id":"20f612d529d077cc48714f87b5ea82e2878df405","unresolved":false,"context_lines":[{"line_number":419,"context_line":"        if type:"},{"line_number":420,"context_line":"            order_type \u003d self._order_type_to_class_map.get(type.lower())"},{"line_number":421,"context_line":"            if order_type is not None:"},{"line_number":422,"context_line":"                return order_type(self._api, **kwargs)"},{"line_number":423,"context_line":"        else:"},{"line_number":424,"context_line":"            raise TypeError(\u0027Unknown Order type \"{0}\"\u0027.format(type))"},{"line_number":425,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9a1a9d01_401c12c1","line":422,"in_reply_to":"9a1a9d01_18cb0655","updated":"2015-09-28 00:05:13.000000000","message":"I\u0027m suggesting:\n\n// type checking\nif not type:\n    raise TypeError(\u0027unknown order type \"{0}\"\u0027.format(type))\norder_type \u003d self._order_type_to_class_map.get(type.lower())\nif not order_type:\n    raise TypeError(\u0027unknown order type \"{0}\"\u0027.format(type))\n\n// business logic\nreturn order_type(self._api, **kwargs)\n\nSo the type checking is first and then the business stuff is really to return the type.\n\nIt\u0027s a small thing. If you don\u0027t agree then it\u0027s ok. Just a personal style that I like to use. Let me know what you think. If you want to keep your style that is ok. Just let me know, and I will revisit.","commit_id":"fc0f7d13bb07145cfde3ac4a858bd37494b794ff"},{"author":{"_account_id":16046,"name":"Fernando Diaz","email":"diazjf@us.ibm.com","username":"diazjf"},"change_message_id":"836c32bc2b4efcecec8caf8cbbadac88c76675fc","unresolved":false,"context_lines":[{"line_number":419,"context_line":"        if type:"},{"line_number":420,"context_line":"            order_type \u003d self._order_type_to_class_map.get(type.lower())"},{"line_number":421,"context_line":"            if order_type is not None:"},{"line_number":422,"context_line":"                return order_type(self._api, **kwargs)"},{"line_number":423,"context_line":"        else:"},{"line_number":424,"context_line":"            raise TypeError(\u0027Unknown Order type \"{0}\"\u0027.format(type))"},{"line_number":425,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9a1a9d01_2d377be4","line":422,"in_reply_to":"9a1a9d01_401c12c1","updated":"2015-09-28 14:57:06.000000000","message":"Hey Nathan, I\u0027ll use your style. Seems more neat to have the return at bottom not enclosed by anything. I just changed up the errors a bit to make more sense to the user,","commit_id":"fc0f7d13bb07145cfde3ac4a858bd37494b794ff"},{"author":{"_account_id":16046,"name":"Fernando Diaz","email":"diazjf@us.ibm.com","username":"diazjf"},"change_message_id":"d75d23a2d115c40241f8e42400c39349c1ab16fa","unresolved":false,"context_lines":[{"line_number":419,"context_line":"        if type:"},{"line_number":420,"context_line":"            order_type \u003d self._order_type_to_class_map.get(type.lower())"},{"line_number":421,"context_line":"            if order_type is not None:"},{"line_number":422,"context_line":"                return order_type(self._api, **kwargs)"},{"line_number":423,"context_line":"        else:"},{"line_number":424,"context_line":"            raise TypeError(\u0027Unknown Order type \"{0}\"\u0027.format(type))"},{"line_number":425,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9a1a9d01_18cb0655","line":422,"in_reply_to":"9a1a9d01_704a6d85","updated":"2015-09-25 19:45:45.000000000","message":"Hello Nathan, Sounds good, but my concern in  this case is that there would need to be multiple error checks if this is done.\n\nEx.)\nif not type:\n    raise TypeError(\u0027Unknown Order type \"{0}\"\u0027.format(type))\norder_type \u003d self._order_type_to_class_map.get(type.lower())\nif order_type is not None:\n    return order_type(self._api, **kwargs)\nelse:\n    raise TypeError(\u0027Unknown Order type \"{0}\"\u0027.format(type))\n\nI can do a try/catch, but the same would occur:\ntry:\n    order_type \u003d self._order_type_to_class_map.get(type.lower())\nexcept AttributeError:\n    raise TypeError(\u0027Unknown Order type \"{0}\"\u0027.format(type))\n\nif order_type:\n    return order_type(self._api, **kwargs)\nelse:\n    raise TypeError(\u0027Unknown Order type \"{0}\"\u0027.format(type))\n\n^^ The other check is needed because If I pass in --type cat, since it is not a valid type order_type will be None after passing the try/catch and then we would run into the issue of \u0027NoneType\u0027 object is not callable, without the second check.","commit_id":"fc0f7d13bb07145cfde3ac4a858bd37494b794ff"},{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"ab07a54e743c0734728c89dcfcd8acb9505586d8","unresolved":false,"context_lines":[{"line_number":417,"context_line":""},{"line_number":418,"context_line":"    def create(self, type\u003dNone, **kwargs):"},{"line_number":419,"context_line":"        if not type:"},{"line_number":420,"context_line":"            raise TypeError(\u0027No Order type Provided\u0027)"},{"line_number":421,"context_line":"        order_type \u003d self._order_type_to_class_map.get(type.lower())"},{"line_number":422,"context_line":"        if not order_type:"},{"line_number":423,"context_line":"            raise TypeError(\u0027Unknown Order type \"{0}\"\u0027.format(type))"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_7680e046","line":420,"updated":"2015-09-28 15:59:22.000000000","message":"nit - lowercase the P in provided","commit_id":"83f4b044ed7ca91df2687bb3f533e443a5053719"},{"author":{"_account_id":16046,"name":"Fernando Diaz","email":"diazjf@us.ibm.com","username":"diazjf"},"change_message_id":"b6e3f9387b2a842b50ef8ff07db8c05c4611a22a","unresolved":false,"context_lines":[{"line_number":417,"context_line":""},{"line_number":418,"context_line":"    def create(self, type\u003dNone, **kwargs):"},{"line_number":419,"context_line":"        if not type:"},{"line_number":420,"context_line":"            raise TypeError(\u0027No Order type Provided\u0027)"},{"line_number":421,"context_line":"        order_type \u003d self._order_type_to_class_map.get(type.lower())"},{"line_number":422,"context_line":"        if not order_type:"},{"line_number":423,"context_line":"            raise TypeError(\u0027Unknown Order type \"{0}\"\u0027.format(type))"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_21d8e4ee","line":420,"in_reply_to":"9a1a9d01_7680e046","updated":"2015-09-28 16:22:49.000000000","message":"Done","commit_id":"83f4b044ed7ca91df2687bb3f533e443a5053719"}]}