All tangled up in IF and ELIF and TRY

I'm working on a small program to add or remove certain entries from the Windows registry, and I'm getting all tangled up in IF and TRY conditions. I've spent hours chasing bugs, usually logical ones, trying to accomplish what is conceptually very simple. The more I work on and "improve" the code so that it works in more and more conditions, the more tortured and complicated the IF tests get, until I can't tell which is up and down anymore. Surely there must be a better way! (even for my little brain)

This is all I'm trying to do:

When asked to install:

If our python version and installpath is in registry: do nothing.

If our python version and different installpath: do nothing.

If our python is not there, add it.

When asked to remove:

If our python is not there: do nothing

If our python version and different installpath: do nothing.

If our python version and installpath is in registry: remove.

But my code for just the removal part looks like the below (the whole thing is here). It works, more or less, but it just doesn't feel right. I don't know what to do to make it cleaner. Your viewpoints are appreciated. Thanks.

```
def remove():
''' see if any existing registrations match our python version and register ours if not '''

if CurrentUser:
match = True if our_version in CurrentUser else False
versions = CurrentUser
elif AllUsers:
match = True if our_version in AllUsers else False
versions = AllUsers
else:
print '\nOur version (%s) not registered to "%s", skipping...' % (our_version, versions[our_version])

try:
if match:
print '\nVersion matches ours, calling deRegisterPy...'
deRegisterPy(pycore_regpath,our_version)
except:
raise

def deRegisterPy(pycore_regpath, version):
''' remove this python install from registry '''
pycore_regpath = pycore_regpath + version # e.g. 'SOFTWARE\Python\Pythoncore\2.7'
try:
reg = OpenKey(HKEY_LOC

def remove():
    ''' see if any existing registrations match our python version and register ours if not '''

    if CurrentUser:

The python style guide reserves CamelCase for class names. If this is a global constant it should be CURRENT_USER. If its not a global constant, it shouldn't be accessed as a global variable.

match = True if our_version in CurrentUser else False

Just use match = our_version in CurrentUser

versions = CurrentUser
    elif AllUsers:
        match = True if our_version in AllUsers else False
        versions = AllUsers

    else:
        print '\nOur version (%s) not registered to "%s", skipping...' % (our_version, versions[our_version])

    try:
        if match:
            print '\nVersion matches ours, calling deRegisterPy...'
            deRegisterPy(pycore_regpath,our_version)
    except:
        raise

What??? the try..except does nothing if you just raise the error right away

def deRegisterPy(pycore_regpath, version):

Python style guide recommends lower_case_with_underscores for functions names

''' remove this python install from registry '''
    pycore_regpath = pycore_regpath + version   # e.g. 'SOFTWARE\Python\Pythoncore\2.7'
    try:
        reg = OpenKey(HKEY_LOCAL_MACHINE, pycore_regpath)
        installpath = QueryValue(reg, installkey) # win32

Why are you taking the time to point out that its win32 here?

if installpath == our_installpath:
            print '\nexisting python matches ours, removing...\n'
            # print '(%s vs %s)' % (installpath, our_installpath)

Don't leave dead code in comments

for subkey in ['\\InstallPath', '\\PythonPath']:
                DeleteKey(HKEY_LOCAL_MACHINE, pycore_regpath + subkey)
            DeleteKey(HKEY_LOCAL_MACHINE, pycore_regpath)
            print "--- Python %s, %s is now removed!" % (our_version, our_installpath)
        CloseKey(reg)
    except EnvironmentError:
        print 'EnvironmentError', EnvironmentError()

What are you trying to do here? You construct a new EnviromentError object, and then print it. But that's a pointless thing to do

raise

If you are just going to raise the error again, not a whole lot of point in printing it anyways
return

Do you realize that nothing after the raise will be executed, so this return will have no effect?

except WindowsError:
        print "Strange, we've hit an exception, perhaps the following will say why:"
        print WindowsError()

You seem to be confused about how to catch errors, you want:

except WindowsError as error:
    print error

But really you don't need to do that: just let the exceptions fly and python will print them when it exits due to the error

raise
        return

Again, the return will never be reached

CloseKey(reg)

If an exception occurs, this will never be called. You should put it in a finally block or something

# main
if args['action']=='install':
    install()
elif args['action']=='remove':
    remove()

You should really put logic like this in a main function.

Any my rewrite of your whole linked file. No testing has been done. But it should give you something of an idea of what you can do.

```
#!/usr/bin/env python

import sys
from _winreg import *

import argparse, sys
import os.path

PYTHON_VERSION = "%d.%d" % sys.version_info[0:2]

# the registry key paths we'll be looking at & using
PYCORE_REGISTRY_PATH = ("SOFTWARE","Python","Pythoncore")
INSTALL_KEY = "InstallPath"
PYTHON_KEY = "PythonPath"
PYTHONPATH = ";".join( os.path.join(sys.prefix, sub) for sub in ["", "Lib", "DLLs"])

class RegisteryKeyNotFound(Exception):
pass

class RegisteryKey(object):
def __init__(self, hive):
self._key = key

def _path_from(self, segments):
if not isinstance(segments, tuple):
segments = (segments,)

return "\\".join(segments)

def __getitem__(self, segments):
try:
subkey = _winreg.OpenKey(self._key, self._path_from(segments))
except WindowsError:
# should really check the error
# to make sure that key not found is the real problem
raise RegisteryKeyNotFound(path)
return RegisteryKey(subkey)

def get_or_create(self, segments):
subkey = _winreg.CreateKey(self._key, self._path_from(segments))
return RegisteryKey(subkey)

def value(self, segments):
return _winreg.QueryValue(self._key, self._path_from(segments))

def delete(self, segments):
_winreg.DeleteKey(self._key, self._path_from(segments))

def set_value(self, segments, value):
_winreg.SetValue(self._key, self._path_from(segments), REG_SZ, value)

def __iter__(self):
index = 0
while True:
try:
yield RegisteryKey(EnumKey(key, index))
except WindowsError:
# add check to make sure correct here was gotten
bre

StackExchange Code Review Q#5217, answer score: 2