Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.python > #99931 > unrolled thread

Re: how to make the below code look better

Back to article view | Back to comp.lang.python

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: how to make the below code look better Ganesh Pal <ganesh1pal@gmail.com> - 2015-12-03 11:50 +0530

#99931 — Re: how to make the below code look better

On Wed, Dec 2, 2015 at 6:00 PM, Chris Angelico <rosuav@gmail.com> wrote:
> If both the functions return true values, yes. You have an indentation
> error there, but I'm assuming you meant to have the try/except
> indented further.
>

Correct I had meant to have try/except indented further.

>> 2. Can I have a if statement within  if else ? , some how I feel its messy
>
> Certainly you can! However, most (maybe all) of your 'if' statements
> are checking for error conditions, so the easiest solution is to
> simply exit right away (either with sys.exit or by raising an
> exception).

Yes agreed , have included the suggestion

>> 3.  Any other suggestion ? please
>
> The first suggestion I'd make is to avoid the comma syntax for
> exception handling. Replace "except Exception, e:" with "except
> Exception as e:". That's a trivial change of syntax that shouldn't
> affect your code at all; consider it low-hanging fruit.
>
,Are we avoiding  comma syntax because it's a bad convention  or  will
it have any side effects? In fact I have used this comma syntax in
most of my code . I will clean the code latter as it seems working
fine

> The second thing to look at is the way you're 'handling' those errors.
> All you do is log the error and exit. So you can skip the except
> clauses altogether, and just do this:
>
> if not os.path.ismount("/tmp"):
>      sys.exit("/tmp not mounted.")
> if create_dataset() and check_permission():
>     run_full_back_up()
>     run_partial_back_up()
> if not validation_errors(): # CHECK ME
>     sys.exit("Validation failed")
> compare_results()
>
>

Agreed.

> Check the logic of validation_errors, though. If it returns something
> True, does that mean there were errors or there weren't? If it means
> there were errors, then the 'not' seems to me to be wrong; but if it
> means there weren't, then I would rename it "validation_successful"
> rather than "validation_errors".
>
> Also worth checking: If you can't create the data set or the
> permission check fails, what should it do? Should it terminate with an
> error? Currently, it carries on and does the validation, which is
> probably not right. If it ought to terminate straight away, you can
> write the whole program in "fail and bail" style:
>
> if not create_dataset():
>     sys.exit("Data set creation failed")
> if not check_permission():
>     sys.exit("Permission check failed")
>

Thanks for other suggestions also

[toc] | [standalone]

Back to top | Article view | comp.lang.python

csiph-web