Replacing excessive conditionals with a list......maybe?

self.chk_var verifies the type and length of a var and calls a method if correct, or prints an error message if not.
self._chk_var also returns a True if the type and length of the var is correct, False if it is not correct.
Currently, I am ignoring the return values.

Code Snippet,

        nbin=NBin()
        if not self.splice_event_cancel_indicator:
            self._chk_var(bool, nbin.add_flag, "out_of_network_indicator", 1)
            self._chk_var(bool, nbin.add_flag, "program_splice_flag", 1)
            self._chk_var(bool, nbin.add_flag, "duration_flag", 1)
            self._chk_var(bool, nbin.add_flag, "splice_immediate_flag", 1)
            self._chk_var(bool, nbin.add_flag, "event_id_compliance_flag", 1)
            nbin.forward(3)
            if self.program_splice_flag:
                if not self.splice_immediate_flag:
                    self._encode_splice_time(nbin)
            if self.duration_flag:                    
                self._encode_break_duration(nbin)
            self._chk_var(int, nbin.add_int, "unique_program_id", 16)
            self._chk_var(int, nbin.add_int, "avail_num", 8)
            self._chk_var(int, nbin.add_int, "avails_expected", 8)
        self.command_length = len(nbin.bites)
            return nbin.bites

Here’s the problem, I have about one hundred vars to verify, and if any of them are wrong I do Not want to execute the last two lines,


        self.command_length = len(nbin.bites)
            return nbin.bites

There’s already a bunch of conditionals and with so many vars I do Not want to do a bunch of nested if statements like this


    if  self._chk_var(int, nbin.add_int, "unique_program_id", 16):
        if self._chk_var(int, nbin.add_int, "avail_num", 8):
            if self._chk_var(int, nbin.add_int, "avails_expected", 8):

My best idea so far is something like,


        if not self.splice_event_cancel_indicator:

             # Check if False is in the list of return values

            if False in [
            self._chk_var(bool, nbin.add_flag, "out_of_network_indicator", 1),
            self._chk_var(bool, nbin.add_flag, "program_splice_flag", 1),
            self._chk_var(bool, nbin.add_flag, "duration_flag", 1),
            self._chk_var(bool, nbin.add_flag, "splice_immediate_flag", 1),
            self._chk_var(bool, nbin.add_flag, "event_id_compliance_flag", 1),
            ]:
                return False
            nbin.forward(3)
            if self.program_splice_flag:
                if not self.splice_immediate_flag:
                    self._encode_splice_time(nbin)
            if self.duration_flag:                    
                self._encode_break_duration(nbin)
            
            # Check if False is in the list
            
            if False in [    
            self._chk_var(int, nbin.add_int, "unique_program_id", 16),
            self._chk_var(int, nbin.add_int, "avail_num", 8),
            self._chk_var(int, nbin.add_int, "avails_expected", 8),
            ]:
                return False
        self.command_length = len(nbin.bites)
            return nbin.bites

Does anyone have a better idea?

How about this:

if not self.splice_event_cancel_indicator:
    checks =
        (bool, nbin.add_flag, "out_of_network_indicator", 1),
        (bool, nbin.add_flag, "program_splice_flag", 1),
        (bool, nbin.add_flag, "duration_flag", 1),
        (bool, nbin.add_flag, "splice_immediate_flag", 1),
        (bool, nbin.add_flag, "event_id_compliance_flag", 1),
    ]

    if not all(self._chk_var(*args) for args in checks):
        return False

    nbin.forward(3)
    if self.program_splice_flag:
        if not self.splice_immediate_flag:
            self._encode_splice_time(nbin)
    if self.duration_flag:                    
        self._encode_break_duration(nbin)

    checks = [
        (int, nbin.add_int, "unique_program_id", 16),
        (int, nbin.add_int, "avail_num", 8),
        (int, nbin.add_int, "avails_expected", 8),
    ]

    if not all(self._chk_var(*args) for args in checks):
        return False
self.command_length = len(nbin.bites)
    return nbin.bites

I like that, it does come out a cleaner and more readable.

If I put

   if not all(self._chk_var(*args) for args in checks):
        return False

in a method like

def check_these(self,checks):
    if not all(self._chk_var(*args) for args in checks):
        return False```

I can do

    checks = [
        (int, nbin.add_int, "unique_program_id", 16),
        (int, nbin.add_int, "avail_num", 8),
        (int, nbin.add_int, "avails_expected", 8),
    ]
  if not self.check_these(checks):
      return False

Thank you man, I like it.

A further change would be to have, say:

    checks =
        (bool, "out_of_network_indicator", 1),
        (bool, "program_splice_flag", 1),
        (bool, "duration_flag", 1),
        (bool, "splice_immediate_flag", 1),
        (bool, "event_id_compliance_flag", 1),
    ]

and use a dict to tell you which of the nbin.add_* methods should be called:

add_method = {bool: nbin.add_flag, int: nbin.add_int}

assuming that all of the bool and int checks will be using the appropriate method.

Just a thought!

three approaches to refactor this come to my mind:

1: use early returns
example:

def f(...) -> Error | int:
  if not self._chk_var(bool, nbin.add_flag, "out_of_network_indicator", 1):
    return Error("out of network indicator not bool")
  # more checks
  self.command_length = len(nbin.bites)
    return nbin.bites

2: use lists “more appropriately”
example:

bool_vars = ["out_of_network_indicator", "program_splice_flag", ...]
if any(self._chk_var(bool, nbin.add_flag, var_name, 1) for var_name in bool_vars): ...

3: raise errors

All of these techniques will avoid nesting a million if statements.

2 is what I wanted to do and it would work for boolean values , because they are all 1 bit, but I also have int, hex, and byte values of various lengths.

I’m going to think about 1 and 3 though.

Thank you for taking the time, I appreciate it.

I didn’t even think about that, you think since they have names like “add_int”
I would have thought of that, but I didn’t.

I like that, because I really hate how long the calls to _chk_var are, a lot of the variable names are so long but they are from a spec so I thought it best to keep them as the come.