0

Is there any way to optimise my if else condition in python?

if not is_text or not is_archive or not is_hidden or \
      not is_system_file or not is_xhtml or not is_audio or not is_video: 
                is_unrecognised = True
else: 
      #get the value which is True out of 
      #is_text,  is_archive,  is_hidden, is_system_file,
      #is_xhtml, is_audio, is_video

I'm feeling that this could be bad idea to write the if condition in this way. So I divided them in to the function and written the code. But Can we optimise my this code to feel it readable and efficient?

Laxmikant Ratnaparkhi
  • 4,745
  • 5
  • 33
  • 49
  • 6
    read any() and all() functions – Grijesh Chauhan Mar 07 '14 at 09:30
  • see Demorgans theorem http://en.wikipedia.org/wiki/De_Morgan%27s_laws – Vorsprung Mar 07 '14 at 09:33
  • http://en.wikipedia.org/wiki/De_Morgan's_laws – Grijesh Chauhan Mar 07 '14 at 09:33
  • 4
    There is no loop here. – tripleee Mar 07 '14 at 09:35
  • 1
    Your current version is almost certainly not correct, unless you really want to only consider the thing recognized if it is text, an archive, hidden, a system file, xhtml, audio, and video all at once. (Why is `is_hidden` even in there, anyway? Does whether a file is hidden or not really have any bearing on whether you recognize it?) – user2357112 Mar 07 '14 at 09:40
  • What do you mean by "get the value which is `True`"? The value, trivially, is `True`, but I imagine you want somehow an indication of which one out of your list of variables was true. It is not very obvious in which representation it would be useful; finding and returning the variable itself is cumbersome, and returning the variable's name as a string is awkward for other reasons. What are you actually trying to accomplish? Would it be better served by a switch-type statement where the first one to be true actually triggers an action of some sort? – tripleee Mar 07 '14 at 09:45
  • @tripleee: Yeah, but there is no switch statement in python, right? – Laxmikant Ratnaparkhi Mar 07 '14 at 10:06
  • Not as such, but @Gassa's answer shows you how to do that in Python. – tripleee Mar 07 '14 at 10:08
  • @user2357112: It is nothing to do with the system file.In our project we assume some db record as a file. and we are doing same operations over that record which is as same as filesystem! – Laxmikant Ratnaparkhi Mar 07 '14 at 10:08
  • @tripleee: Gassa is right but nesting is not required there! That's what I wanted to say. – Laxmikant Ratnaparkhi Mar 07 '14 at 10:11
  • There is no nesting there. – tripleee Mar 07 '14 at 10:11
  • @tripleee: Hello triplee, Isn't it nested if else there? – Laxmikant Ratnaparkhi Mar 07 '14 at 10:14

8 Answers8

4

  1. Read De Morgan's laws:

    "not (A and B)" is the same as "(not A) or (not B)"

  2. all(iterable): Return True if all elements of the iterable are true (or if the iterable is empty).
    any(iterable): Return True if any element of the iterable is true. If the iterable is empty, return False 

    conditions = (  # make a tuple: an iterable
   is_text, 
   is_archive, 
   is_hidden, 
   is_system_file, 
   is_xhtml, 
   is_audio, 
   is_video
 )

 if not all(conditions): # apply De Morgans
     is_unrecognised = True
 else:
    # other code

Btw, if it meets to your requirement, it can be further simplified without if as 

is_unrecognised = not all(conditions)
# may be you can write
# if not is_unrecognised:
#    you code in else
Grijesh Chauhan
  • 57,103
  • 20
  • 141
  • 208
  • Is (a) creating an iterable tuple, (b) iterating it, and (b) calling 'any' or 'all' really an optimization? The code is of course more readable. But, is it really optimized? – wolfrevo Mar 07 '14 at 12:27
  • @wolfrevo may be creating tuple cause extra overhead but if you are thinking of short-circuiting then [yes `any()`, `all()` supports short-circuiting and stops evaluating as soon as result evaluated](http://stackoverflow.com/a/14892812/1673391). – Grijesh Chauhan Mar 07 '14 at 12:35
  • 1
    Yes, I meant that. Thanks for the hint to short-circuiting support! – wolfrevo Mar 07 '14 at 12:41
3

Boolean algebra satisfies De Morgan's laws,

(De Morgan 1) (¬x)∧(¬y) = ¬(x∨y)
(De Morgan 2) (¬x)∨(¬y) = ¬(x∧y).

Thus you could change it to

if not ( is_text and is_archive and is_hidden and is_system_file and is_xhtml and is_audio and is_video):
    is_unrecognised = True
else: 
    #get the value which is True out of 
    #is_text,  is_archive,  is_hidden, is_system_file,
    #is_xhtml, is_audio, is_video
wolfrevo
  • 6,651
  • 2
  • 26
  • 38
3

First of all, I would try to use try: except: than if: else:. It is faster, for a reason that I do not completely understand. The only thing is that your statement have to rise exception.

Another way is to do it like that:

if False in [is_text, is_archive, is_hidden, is_system_file, is_xhtml, is_audio, is_video]:
Andy S. C.
  • 368
  • 1
  • 4
  • 17
3

It looks like you will have to find out exactly which of the conditions is true anyway. In that case, I'd suggest to write it like this:

if is_text: # do something elif is_archive: ... elif is_video: # do something else: is_unrecognised = True

This construction is similar to a switch or case statement in other languages.

Edit: and yes, as suggested in the comments, perhaps your original code should contain ands instead of ors. So that it goes like: if it is (1) not a text and (2) not a video and (...) not all other recognizable things, then it is unrecognized.

Gassa
  • 8,546
  • 3
  • 29
  • 49
  • I don't have to do anything if is_text or is_image or is_audio or is_video .. etc Since nesting is not required here! – Laxmikant Ratnaparkhi Mar 07 '14 at 09:56
  • What's the `#get the value which is True out of...` comment supposed to mean, then? – Gassa Mar 07 '14 at 10:01
  • You could be right! Sorry. But I think anyhow nesting is not required to get any true Value out of those! – Laxmikant Ratnaparkhi Mar 07 '14 at 10:09
  • No, but what do you need the value for? Whatever code needs that answer might make sense to inline here. – tripleee Mar 07 '14 at 10:10
  • 1
    What I'm going to say is a bit speculative, but even if you are just assigning a variable, like "if is_text: type = "TEXT" elif is_archive: type = "ARCHIVE" else ...", such an enumeration of cases (call it nesting if you wish) makes sense to future reader. Usually, the code does not have to look clever, it just has to be readable. Sure, it is long-ish though. – Gassa Mar 07 '14 at 10:16
1

I think the shortest and Pythonic wa to do it is some modification from the @Grijesh answer. Namely,

conditions = (
    is_text, 
    is_archive, 
    is_hidden, 
    is_system_file, 
    is_xhtml, 
    is_audio, 
    is_video
)

is_unrecognised = True if not any(conditions) else False
Javier
  • 1,027
  • 14
  • 23
1

What everyone else has said so far is correct, that you can use De Morgan's laws and rewrite the if test.

However, as far as I can see the biggest problem at the moment is that if I understand your code correctly, it doesn't do what you think it does.

What you actually want seems to be either

if not (is_text or is_archive or is_hidden or \
  is_system_file or is_xhtml or is_audio or is_video): 
            is_unrecognised = True

or

if not any(is_text, is_archive, is_hidden, is_system_file, is_xhtml, is_audio, is_video): 
            is_unrecognised = True

Edit: Or Gassa's solution which seems to be even more in line with what you are actually trying to do.

Erik Vesteraas
  • 4,675
  • 2
  • 24
  • 37
1

Your data representation may be leading you astray here. What if instead you did something like

tests = dict()
testkeys = ['text', 'archive', 'hidden', 'system', 'xhtml', 'audio', 'video']
[tests[i] = False for i in testkeys]

# whatever code you have now to set is_text should now instead set tests['text'], etc

if not True in tests.values():
    is_unrecognized = True
else:
    recognized = [k for k in tests if tests[k] is True]  # f'rinstance

(I'm sure there are more idiomatic ways of doing this. Just suggesting a different mode of thinking, really.)

tripleee
  • 175,061
  • 34
  • 275
  • 318
1

Even though this question has already been answered. I present an alternative which covers a comment in the code of the question.

As pointed out by @tripleee the original question asked in the code comment to get the variables with value True.

@tripleee's answer asumes creating a dictionnary with the variable names.

An alternative to it is to get the names from locals. The following code 

is_text = False
is_archive = True
is_hidden = False
is_system_file = False
is_xhtml = False
is_audio = True
is_video = False
the_size = 34 # <- note also this
conditions = (
               is_text,
               is_archive,
               is_hidden,
               is_system_file,
               is_xhtml,
               is_audio,
               is_video,
               the_size,
               )

result = [(k, v) for k, v in list(locals().iteritems()) \
          if id(v) in [id(condition) for condition in conditions] and v]

if len(result) == 0:
    is_unrecognized = True
else:
    # do what you want with your variables which have value True
    # as example print the variable names and values
    for k, v in result:
        print 'variable "%s" is %s' % (k, v)
wolfrevo
  • 6,651
  • 2
  • 26
  • 38