refactor #1
Open
refactor #1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 19 commits
September 27, 2014 10:31
avoid depending on python's outer to inner scope unless necessary, doing so limits modularity (e.g. now you could call 'start()' with an entirly different serial object from another location in the program
same reason as last commit
Whenever managing a resource that requires an open/close, such as a database, follow the [RAII] idiom. Loosly speaking, close the resource in the same scope as it was opened. Here I use the try/finally pattern to ensure that db.close() is called even if an exception is thrown during execution of 'start(...)'. Later I'll refactor the DB connection code out into its own object so that we can do something like with open_connect(...) as cursor: # use cursor
advantages of this are the call and responses are all together in a dictionary, makes it easier to eyeball for correctness, and now just one call to log.write(...) (less repeated code)
same reason as before
collecting the common insertion code into a single object makes it easier to reuse that code in the future. It is also only one line of actual SQL that needs to be debugged
note, only one occurace of db.commit() now
getting confused with ruby syntax!
encapsulate database connection the __enter__ and __exit__ functions allow this object to be used with the "with ... as val" pattern
oops, python, not C++ or Java
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
the final changes look quite extensive, but I committed each incremental change with a message explaining my reasoning. I haven't had a chance to test this with actual data, so pull into a new branch and do some testing before merging to master.
Related to that, there should be some sample data for each scenario in the repo in a 'test' directory or something for testing purposes.