- I don't see why
del jsonobject[...]
would be necessary before reassigning
reassigning that key. Also, the test in the last two lines could also
also be simplified to something like
jsonobject.pop('small_encoded_polyline', None)
if you don't care about
about that key, no?
Also:
- Everything could use more error checking. That is, check expected
types and values instead of falling through the
if
s ('imperial'
,
'metric'
, what happens if it's neither, or both?).
- Unless you really prefer
list
and dict
, it's better to use the
literal syntax (fast, can't override it).
- The type check for
dict
s can be better, i.e.
isinstance(foo, dict)
; if you really want only dict
, then it's
still better to use type(foo) is dict
instead of comparing strings.
I'd also move that whole if/else
into a function,
e.g. maybe_load_json
or so.
- The value of
document
after the assignments isn't used anywhere, so
it's safe to remove, though I'd rather use that variable again instead
of introducing document2
.
- The second branch with
'small_encoded_polyline'
seems to have the
wrong level of indentation? At least it's different from the first
one in that it's only run if the document wasn't a dict
. Assuming
that it's safe to run anyway I'll change that in the code below.
- The pattern
if foo in json and json[foo] is not None:
could be easier
be achieved with if json.get(foo) is not None:
as the return value
there defaults to None
.
- I'm not particularly fond of the habit to reuse input data like with
'points_correct'
done here. It would be cleaner just to have a list
points_correct
and assign that to a jsonobject
key (and you know,
possibly put the calculation into a separate function as well).
I might have messed up some of the logic now, but I hope you get the
idea:
def maybe_load_json(document):
if isinstance(document, dict):
return document
return json.loads(document)
def cleaned_small_encoded_polylines(documents):
result = []
for document in documents:
#sometimes these document objects turn out to be dicts
#and sometimes they turn out to be strings
#even though JSON output is always the same
#is there a better way to deal with this?
document = maybe_load_json(document)
if document.get('small_encoded_polyline') is not None:
del document['small_encoded_polyline']
#I thought this line above would modify the original
#jsonobject since document is a dictionary so I should
#be working with a pointer to the original object
#but inspection of jsonobject reveals this not to be the case
result.append(document)
return result
...
r = requests.get(url, headers = {'User-Agent':UA})
jsonobject = r.json()
if jsonobject.get('laps') is not None:
if 'imperial' in jsonobject['laps']:
laps_array = jsonobject['laps']['imperial']
type = 'imperial'
if 'metric' in jsonobject['laps']:
laps_array = jsonobject['laps']['metric']
type = 'metric'
if laps_array is not None:
jsonobject['laps'][type] = cleaned_small_encoded_polylines(laps_array)
# this can't be an elif because sometimes json objects
# have both "points" and "laps"
if jsonobject.get('points') is not None:
if 'points' in jsonobject['points']:
points_correct = []
laps_array = jsonobject['points']['points']
if laps_array is not None:
jsonobject['points']['points'] = cleaned_small_encoded_polylines(laps_array)
jsonobject.pop('small_encoded_polyline', None)