Suggestions about improving my code

Question

yesterday I have found that I can't use the wireless network at some spots in my house. I used another modem as a WiFi booster and I managed to cover these spots.

The problem is that when I go to these dead spots I need to use static IP and change my primary dns servers, or I get limited connection. Also, I still want to use DHCP when I'm not in these spots.

I have written two batch files and a python script to define the wireless adapter settings.

I would like someone to take a look and suggest how to improve it.

Batch Files (I'm using shortcuts because of the option to run them as administrator)

DeadSpots.bat.ink

netsh interface ip set address "Wi-Fi" static 192.168.x.x 255.255.255.0 192.168.x.x
netsh interface ip set dns "Wi-Fi" static 192.168.x.x primary # This is the second modem
netsh interface ip add dns "Wi-Fi" ISP.dns.IP index=2

Regular.bat.ink

netsh interface ip set address "Wi-Fi" dhcp
netsh interface ip set dnsservers "Wi-Fi" source=dhcp

Python code

import subprocess as sub

def WiFi():
    filepath1 = Path_To_DeadSpots.bat.ink
    filepath2 = Path_To_Regular.bat.ink
    loc = input("Please choose your location: 1-Rooms, 2-Rest \n")
    while(loc != "1" and loc != "2"):
        print("Wrong input, please choose again")
        loc = input("Please choose your location: 1-Rooms, 2-Rest \n")
    if loc == "1":
        p = sub.Popen(filepath1,shell=True,stdout=sub.PIPE)
    else:
        p = sub.Popen(filepath2,shell=True,stdout=sub.PIPE)
WiFi()

Please suggest improvements, thank you.

If your code works and does what you want it to do, you could take it to [Code Review](http://codereview.stackexchange.com/) for suggestions on ways to improve it. But *only if it works* -- they tend to get very annoyed if they find they have to debug code as well (that's more our job here on SO.) — DSM, Sep 24 '15 at 18:31
@DSM, thanks, I will ask there. wOxxOm, thank you for editing my post. — Galc127, Sep 24 '15 at 18:34
@DSM Actually, for clarification, we don't get annoyed. Broken code is strictly off-topic. That doesn't mean that an obscure bug renders a question off-topic. But if it is obviously broken (doesn't do what it is supposed to, doesn't compile even in the best-case-scenario, there's a syntax error... things you can catch if you run your own code) is closed. — Ismael Miguel, Sep 24 '15 at 18:36
I don't know python but there is no goto if error? seems to me `loc = input` are redundant — Paul, Sep 24 '15 at 18:36
Guys stop this is a place for people to ask questions and learn programming... If he's asking for improvements, give him improvements! — visc, Sep 24 '15 at 18:49
@JeffreyHaines: that's far too broad a brush, and not how we operate. By the "ask questions and learn programming" standard, we'd almost never close anything — DSM, Sep 24 '15 at 19:53
@DSM I don't see a purpose to the dichotomy you have in mind... Perhaps you should be helping people more and not fuel spats Dr. — visc, Sep 24 '15 at 20:18

visc · Accepted Answer · 2015-09-24T20:14:12.800

Not knowing the rest of your programs structure...

You could spice it up with argparse!

Then you can call:

python wifi.py --path-to-deadspots ./deadspots.bat.ink --path-to-regulars ./regulars.bat.ink --room-loc 2

Example:

import argparse
import subprocess as sub

class IsAccesible(argparse.Action):
    def __call__(self, parser, namespace, values, option_string=None):
        if os.path.isfile(values):
            if os.access(values, os.R_OK):
                setattr(namespace, self.dest, values)
            else:
                raise ValueError("Path is not accesible")

class IsValidRoomLoc(argparse.Action):
    def __call__(self, parser, namespace, values, option_string=None):
        if values == 1 or values == 2:
            setattr(namespace, self.dest, values)
        else:
            raise ValueError("Room loc is not valid")

def WiFi():

    parser = argparse.ArgumentParser()
    parser.add_argument('--path-to-deadspots', dest='path_to_deadspots', help="The deadspots file location", type=str, required=True, action=IsAccesible, default="./deadspots.bat.ink")
    parser.add_argument('--path-to-regular', dest='path_to_regular', help="The regular file location", type=str, required=True, action=IsAccesible, default="./regular.bat.ink")
    parser.add_argument('--room-loc', dest='room_loc', help="The room lock, \'1\' or \'2\'", type=int, required=True, action=IsValidRoomLoc, default=1)

    args = parser.parse_args()

    path_to_deadspots = args.path_to_deadspots
    path_to_regular = args.path_to_regular
    room_loc = args.room_loc

    if room_loc == "1":
        p = sub.Popen(path_to_deadspots,shell=True,stdout=sub.PIPE)
    else if room_loc == "2":
        p = sub.Popen(path_to_regular,shell=True,stdout=sub.PIPE)

WiFi()

You can also remove the the first argument and just have:

python wifi.py ./deadspots.bat.ink ./regulars.bat.ink 2

You're welcome :-) Just learned how to do that in the past few weeks! — visc, Sep 24 '15 at 18:50
`if values is 1 or values is 2`: `is` tests *identity*. You want to test equality. This only works because of an implementation detail of CPython. See [here](http://stackoverflow.com/questions/132988/is-there-a-difference-between-and-is-in-python) for a more detailed explanation. — DSM, Sep 24 '15 at 19:55

Suggestions about improving my code

1 Answers1