[Tutor] Code Review?

[Brian Carpio]

Thanks to everyone who has replied! This is some good information for me to
go learn with!.

I greatly appreciate it.

Brian

On Mon, Aug 6, 2012 at 9:21 AM, Peter Otten <__peter__ at web.de> wrote:

> Brian Carpio wrote:
>
> > Hi,
> >
> > Hopefully I am allowed to ask this here. I am pretty new to python I've
> > only been writing code for about 6 months now strictly for system
> > administration purposes; however I have now decided to write something
> > "real" that others might benefit from but I am looking for someone to
> take
> > a look at my code and give me an idea on some areas where I need
> > improvement. I certainly don't expect anyone to rewrite my code but if
> > someone is willing to give it a quick look and tell me things like focus
> > more on XX or you really need to learn YY because its obvious you don't
> > have a clue (lol) those types of comments would be more then welcome.
> >
> > I've ran my code through pylint and am getting a score of 7-9 with all of
> > my scripts, but a program can only tell you so much having a real person
> > point out some of my shortcomings would be amazing. Sometimes you don't
> > know you don't know something until someone tells you "Hey you don't know
> > XX or YY go learn it".
> >
> > https://github.com/bcarpio/mongodb-enc
>
> [I took a look at mongodb_node_classifier.py]
>
> Random remarks:
>
> - pylint probably "told" you: use self-explanatory names rather than i, d,
> or n.
> - Split your code in smaller dedicated functions. This simplifies writing
> unittests, allows reuse and keeps control flow manageable as your script
> grows.
> - Add a comment that explains what the script does; repeat for each
> function.
> - Use main() to read the configuration/commandline and put the real meat
> into a dedicated function.
> - Prefer raising exceptions over sys.exit() in the code that does the real
> work (main() may convert these into sys.exit).
>
> Here's a sketch for a revised main():
>
> def main():
>     node_name = sys.argv[1]
>     filename = ...
>     conn_spec = read_config(filename)
>     connection = open_connection(con_spec)
>     try:
>         process(connection, node_name)
>     except InheritanceError as err:
>         sys.exit(str(err))
>
> (You should choose a name that is more descriptive than process())
>
> - Use 'expr', not 'expr == True' in boolean contexts
> - Show that you know None is a singleton and prefer 'expr is None' over
> 'expr == None'
> - I found the while loop hard to understand. I have a hunch that it can be
> simplified.
>
> _______________________________________________
> Tutor maillist  -  Tutor at python.org
> To unsubscribe or change subscription options:
> http://mail.python.org/mailman/listinfo/tutor
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/tutor/attachments/20120806/9f0983c6/attachment.html>