6
\$\begingroup\$

I am using the library geoip2 to get Geolocation of many IP adderesses


"""
input:
        str:   IP
output ordered list: 
    [0] str:   City, State, Country
    [1] tuple: (Lat; Log)
    [2] str:   Postal
"""

for i in pd.unique(df_to_print['requesterIp']):
    res = reader.city(i)
    # NOTE: Second snippet is added here
    myDict[i] = [res.city.names['en'] + ", " + res.subdivisions[0].names['en'] + ", " +  res.country.names['en'],(res.location.latitude, res.location.longitude), res.postal.code]

# output:  ['Calgary, Alberta, Canada', (50.9909, -113.9632), 'T2C']

Sometimes the response, which is in JSON, is missing some fields. This causes a exception.

Here is my proposed "fix", the code works as intended, but looks sinful

try:
    city = res.city.names['en']
except:
    city = "-1" 

try:
    state = res.subdivisions[0].names['en']
except:
    state = "-1" 
    
try:
    country =  res.country.names['en']
except:
    country =  "-1"
    
try:
    cord = (res.location.latitude, res.location.longitude)
except:
    cord = (-1, -1)

postal = res.postal.code if res.postal.code is not None else -1


print([city + ", " + state + ", " +  country, cord, postal]) 
# output: ['-1, -1, China', (34.7725, 113.7266), -1]

What can I do to make my code more professional and efficient?

(this will run for apx. 100K unique IPs, several times a hour; DB is local)

\$\endgroup\$
3
\$\begingroup\$

Some suggestions:

  1. Don't use exceptions for flow control unless you have to for TOCTOU or other reasons. "Tell, don't ask" is a useful guideline when requesting something from a piece of state the current code has no control over, but it's not a law. In your code there is no chance of res.country.names changing while running res.country.names.get('en', "-1"), and that is much clearer than using exception handling to set a default.
  2. You almost always want to catch specific exceptions.
  3. When retrieving values from a dict you can use my_dict.get("key", default) to get a default value if the key does not exist.
  4. Use f-strings rather than + to create formatted strings.
\$\endgroup\$
13
  • 3
    \$\begingroup\$ Could you elaborate on the first point ? Exceptions-flow control etc \$\endgroup\$
    – aki
    Aug 5 '20 at 6:02
  • 2
    \$\begingroup\$ @anki The most common use of exceptions is for handling (or choosing not to handle) errors, failures, or unexpected conditions. If the situation is expected, then it's often better to use some kind of if statement, since exceptions have high overhead, and need to be used carefully to avoid catching unrelated errors. That said, there are situations where exception based flow control is the lesser of two evils, and "asking for forgiveness rather than permission" is sometimes seen as a good approach, although it's less common than it was. \$\endgroup\$
    – James_pic
    Aug 5 '20 at 8:52
  • \$\begingroup\$ @anki the first point is wrong. Using exceptions for control flow is a widely accepted in the python community: even standard and famous libraries are using exceptions for control flow and it's considered to be pythonic. This misconception comes from C++/Java/what else programmers, where exceptions have a lot of overhead and are not really efficient. \$\endgroup\$
    – Gsk
    Aug 5 '20 at 12:47
  • \$\begingroup\$ @Gsk I agree that exceptions should not be used by beginners if they don't know how to use it properly (and this is not about efficiency at all) See this series youtube.com/watch?v=W7fIy_54y-w . I just wanted another viewpoint on this by the answerer here. \$\endgroup\$
    – aki
    Aug 5 '20 at 12:56
  • 2
    \$\begingroup\$ @spyr03 & others, I've updated to explain why exceptions for flow control is a bad idea for this specific code sample. \$\endgroup\$
    – l0b0
    Aug 5 '20 at 21:54

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.