2
\$\begingroup\$

The code uses SQLAlchemy Core. However, what I want to know is if writing code in this pattern is recommendable.

def saveUser(self, user):
    """
    Inserts or updates a user in database.
    The method first checks if the provided user's id already exists. If no, then the user will be inserted. Otherwise, the stored values are updated.

    Args:
        user (User)
    Returns:
        User object with inserted id
    """
    result = self._engine.execute(self._userTable.select().where(self._userTable.c.Id == user._id))
    if not result.first():
        # user doesn't exist in database.
        result = self._engine.execute(self._userTable.insert(), Email=user.email, PasswordHash=user.passwordHash,
                                      DisplayName=user.displayName, Active=user.active)
        user._id = result.inserted_primary_key[0]
    else:
        # user already exists in database.
        result = self._engine.execute(self._userTable.update().where(self._userTable.c.Id == user._id),
                                      Email=user.email, PasswordHash=user.passwordHash, DisplayName=user.displayName, Active=user.active)
    return user


def deleteUser(self, userId=None, email=None):
    """
    Deletes a user.
    Either userId or email can be passed. If both are passed, then a user containing both userId and email will be deleted.
    If none of the arguments are provided, then the method call will have not effect.
    Args:
        userId (int): If this is None, only email will be used to delete the user.
        email (str): If this is None, only userId will be used to delete the user.
    """
    if not userId and not email:
        return # Because an empty and_ clause generated later on will raise OperationalError.
    idClause = self._userTable.c.Id == userId if userId else None
    emailClause = self._userTable.c.Email == email if email else None
    finalClause = and_(idClause, emailClause)
    deleteQuery = self._userTable.delete().where(finalClause)
    result = self._engine.execute(deleteQuery)
\$\endgroup\$
  • 1
    \$\begingroup\$ You don't need to roll your own insert-or-update logic: SQLAlchemy has a merge method. \$\endgroup\$ – Gareth Rees Oct 12 '13 at 13:25
  • \$\begingroup\$ @GarethRees - Isn't that limited to SQLAlchemy ORM? I am using Core here. \$\endgroup\$ – Jayesh Bhoot Oct 13 '13 at 6:19
2
\$\begingroup\$

The only thing I can think of that might make it more Pythonic would be to wrap the body of deleteUser() in a try and catch and handle the OperationalError rather than checking user and email for None before making the attempt.

\$\endgroup\$

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.