8
\$\begingroup\$

My concerns:

  1. Is the code pythonic
  2. Is the code practical

I tend on expanding the usefulness of this script, if you don't think it's stupid idea. One thing I will implement is command-line arguments by way of argparse.

#!/usr/bin/env python


'''
Test if network is online by sending and http request to a remote
web-server. I wrote this because my network connection sucks.
'''

import urllib2
import sys


def is_online(**kwargs):
    '''
    Determine weather or not your network is online
    by sending a HTTP GET request. If the network is
    online the function will return true. If the network
    is not online the function will return false
    '''

    # Set the default request time out.
    # This will be passed as a keyword argument
    # to urllib2.urlopen.
    kwargs.setdefault('timeout', 0.30)
    kwargs.setdefault('url', 'http://www.google.com')
    timeout = kwargs.get('timeout')
    url = kwargs.get('url')

    try:
        print 'Sending request to {}'.format(url)
        response = urllib2.urlopen(url, timeout=timeout)
    except KeyboardInterrupt:
        sys.exit(0)
    except Exception:
        # Generally using a catch-all is a bad practice but
        # I think it's ok in this case
        response = False
        print 'Request to {} failed'.format(url)
    if response:
        return True
    else:
        return False


def main():
    '''
    Continually check if the network is online.
    If the network is online stop checking.
    '''
    while True:
        if is_online():
            print 'Net is up'
            break
        else:
            print 'Net down.'

if __name__ == '__main__':
    main()
\$\endgroup\$

2 Answers 2

7
\$\begingroup\$

I would use Python's socket library instead of urllib. Then, you could test like this:

import socket
REMOTE_SERVER = "www.google.com"
def is_connected():
  try:
    host = socket.gethostbyname(REMOTE_SERVER)
    s = socket.create_connection((host, 80), 2)
    return True
  except:
     pass
  return False
print is_connected()

Borrowed from here


I think the most obvious thing to point out is this:

if response:
    return True
else:
    return False

You can actually just return the variable, so just return response would replace all those lines.


There's not really any reason to use .format for one variable.

'Sending request to {}'.format(url)

Plain string concatenation is fine, for one variable.


Anyway, what's the point of printing out that you're requesting google.com?

'Request to {} failed'.format(url)

The level of abstraction that you put into this, is that it'll return whether you're connected or not.

Printing out anything other than: 'Net is up' or 'Net down' (Which could better be: 'Net is down, retrying connection') seems superfluous.


kwargs.setdefault('timeout', 0.30)

I would avoid the magic numbers (undefined numbers: 0.30), define it as a constant (in all caps) to improve readability.


Is this code Pythonic?

I'd recommend reading PEP8, Python's official style guide. It has many guidelines to follow on just about everything; it should really be a Python programmer's Bible. There's also a nice little Online PEP8 checker to help/show you where your code doesn't meet the standards of PEP8.

The term 'Pythonic' is not so easily defined, so I'm not really going to give you a 'Yes or No' answer, but, I'd just really recommend reading over PEP8.

Is this code practical?

Yes, it's practical.

However, one thing more to point out; While you keep retrying and retrying:

  • Put a some kind of wait in between network connection tests.
  • Every time you call is_online, when the Net is down, you redo the url setup, consider keeping that seperate so you can only do the bare minimum for the continual tests:
kwargs.setdefault('timeout', 0.30)
kwargs.setdefault('url', 'http://www.google.com')
timeout = kwargs.get('timeout')
url = kwargs.get('url')
\$\endgroup\$
2
  • \$\begingroup\$ Nicely done, I especially like the part about the mismatched level of abstraction. Though, I'd say the OP's dependency on urllib2 is not practical, when the job can be done without it \$\endgroup\$
    – janos
    Aug 22, 2015 at 23:39
  • \$\begingroup\$ Is there any other way to check internet running status. Because if you pining google.com again again, then they will block our IP. So Is there any other way.? \$\endgroup\$
    – Sanjiv
    Aug 21, 2019 at 8:33
2
\$\begingroup\$

This try/except block serves no purpose.

try:
    print 'Sending request to {}'.format(url)
    response = urllib2.urlopen(url, timeout=timeout)
except KeyboardInterrupt:
    sys.exit(0)

When the user presses CTRL+c the program will exit. There is no need to catch the error and then manually exit with sys.exit.

\$\endgroup\$
1
  • \$\begingroup\$ Won't it suppress the traceback usually associated with a KeyboardInterrupt exception? \$\endgroup\$
    – Daniel
    Jun 20, 2018 at 22:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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