1
\$\begingroup\$

Below is a function that adds a new user to the database using their email and password. I am using Flask and SQLAlchemy.

@app.route('/users', methods = ['POST'])
@require_appkey
def registerUser():
    errorsList = []
    userInfo = request.get_json()

    # Get "email" and "password"
    email = userInfo.get('email')
    password = userInfo.get('password')     
    if email is None or password is None:                                                                                                                       
        errorsList.append(Error("email/password","Email or Password not entered"))
        return make_error(MainError('400','Incorrect Input',errorsList))

    # Check password meets criteria
    if len(password) > 1024:
        errorsList.append(Error("password","Password maximum length is 1024 characters")) 
    if len(password) < 8:
        errorsList.append(Error("password","Password must be at least 8 characters"))


    # Check email meets criteria
    email = email.strip()
    # Check if email address is valid (syntactically)
    match = re.match("[^@]+@[^@]+\.[^@]+", email)
    if match is None:
        errorsList.append(Error("email","Not a valid email address"))
    # Check if email address is already in database
    with contextlib.closing(DBSession()) as session:        
        try:
            if session.query(USER).filter_by(USERSEMAIL=email).count():
                errorsList.append(Error("email","This email address already exists"))
        except Exception as error:
            session.rollback()
            logger = logging.getLogger('__name__')
            logger.error(error)
            errorsList.append(Error("database","An error occurred with the database. Please try again later."))
            return make_error(MainError('500','Database error',errorsList))


    # If errors exist, return all errors
    if errorsList:
        return make_error(MainError('400','Incorrect Input',errorsList))

    # Add user to database      
    user = USER(email, password)        
    with contextlib.closing(DBSession()) as session:            
        try:
            session.add(user)
            session.commit()
        except Exception as error:
            session.rollback()
            logger = logging.getLogger('__name__')
            logger.error(error)
            errorsList.append(Error("database","An error occurred with the database. Please try again later."))
            return make_error(MainError('500','Database error',errorsList))
        return jsonify(data=user.serialize())

def make_error(mainError):
    response = jsonify(data=mainError.serialize())
    response.status_code = int(mainError.code)
    return response

class MainError:
    def __init__(self, code, message, errorsList):          
        self.code = code
        self.message = message
        # List of Error objects
        self.errorsList = errorsList

def serialize(self):  
    return {           
    'errorCode': self.code, 
    'message': self.message,
    'errorList': [error.serialize() for error in self.errorsList],
    'success': 'false'
    }

My specific questions (not all directly related to Python) are:

  1. Should I be using return in so many different places?

  2. Is there a better way to structure my code? I would love to hear any thoughts you all have on how this could be written better.

  3. Is the way in which I am handling my errors correct?

  4. Seeing as I will probably be using the Database error often, is it worth putting into a function?

\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! :) Could you please fix your indentation? It seems a little bit off at places and makes it hard to know what belongs where. \$\endgroup\$ – Morwenn Mar 30 '15 at 9:51
  • \$\begingroup\$ Why do you return (rather than raise) an error that doesn't even inherit from Exception? And why is make_error separate from the class? \$\endgroup\$ – jonrsharpe Mar 30 '15 at 10:35
1
\$\begingroup\$

MainError can be simplified as follows,

class MainError:
    def __init__(self, code, message, errorsList):          
        self.resp=jsonify(dict(errorCode=code,
                        message=message,
                        errorList=[error.serialize() for error in errorsList],
                        success=False))
        self.resp.status_code = int(code)
    def get_error(self):
        return self.resp

Now an error can be returned by return MainError('400','Incorrect Input',errorsList).get_error()

A simple function can do the above thing,

def get_error(code, message, errorsList):
    resp = jsonify(dict(errorCode=code,
                            message=message,
                            errorList=[error.serialize() for error in errorsList],
                            success=False))
    resp.status_code = int(code)

    return resp

For multiple returns define an error object like error=None and use if and elif and return error if you have errors. f.e,

error = None
if email is None or password is None:                                                                                                                       
        errorsList.append(Error("email/password","Email or Password not entered"))
        error = MainError('400','Incorrect Input',errorsList).get_error()
elif len(password) > 1024:
        errorsList.append(Error("password","Password maximum length is 1024 characters"))
        error = MainError('400','Incorrect Input',errorsList).get_error()
return error if error else jsonify(data=user.serialize())
\$\endgroup\$

Your Answer

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