Refactoring PlaceUri in distributed #3970
Conversation
| if response and response.journeys and response.journeys[0].sections: | ||
| if pt_object_origin.embedded_type == type_pb2.POI: | ||
| response.journeys[0].sections[0].origin.CopyFrom(pt_object_origin) | ||
| if pt_object_destination.embedded_type == type_pb2.POI: | ||
| response.journeys[0].sections[-1].destination.CopyFrom(pt_object_destination) | ||
|
|
There was a problem hiding this comment.
I'm not sure that I understand this piece of code. The reason is: I don't understand why we do have a piece of code to fill up origin and destination that is specific to POI. Wouldn't we have something similar for addresses or stops too?
There was a problem hiding this comment.
No need to do it for other objects
There was a problem hiding this comment.
How about adding a comment, something like this?
# In case of a POI, it might be an access point, but we want the parent POI to be shown on.|
|
||
| pt_object_origin = None | ||
| pt_object_destination = None | ||
| if request.get('origin'): |
There was a problem hiding this comment.
request.get("origin") is called 4 times in the following lines. Maybe putting it in a variable would be interesting.
There was a problem hiding this comment.
Sonar show 2 code smells that probably can be addressed.
https://sonarcloud.io/project/issues?id=Hove_navitia&pullRequest=3970&resolved=false&types=CODE_SMELL
| def get_detail_pt_object(self, instance, arg_pt_object, request_id): | ||
| if not arg_pt_object: | ||
| return None | ||
| detail = self.get_entrypoint_detail(arg_pt_object, instance, request_id="{}".format(request_id)) |
There was a problem hiding this comment.
| detail = self.get_entrypoint_detail(arg_pt_object, instance, request_id="{}".format(request_id)) | |
| detail = self.get_entrypoint_detail(arg_pt_object, instance, request_id=request_id) |
| if response and response.journeys and response.journeys[0].sections: | ||
| if pt_object_origin.embedded_type == type_pb2.POI: | ||
| response.journeys[0].sections[0].origin.CopyFrom(pt_object_origin) | ||
| if pt_object_destination.embedded_type == type_pb2.POI: | ||
| response.journeys[0].sections[-1].destination.CopyFrom(pt_object_destination) | ||
|
|
There was a problem hiding this comment.
How about adding a comment, something like this?
# In case of a POI, it might be an access point, but we want the parent POI to be shown on.
|
Kudos, SonarCloud Quality Gate passed! |
No description provided.