0

I have the following code which does:

  1. check a schedulingStrategy
  2. According to the scheduling strategy
  3. Check the associated scheduling period

For now I only have two scheduling strategy thus my code is pretty straight forward:

def test_scheduling_parameters(schedulingStrategy: str, schedulingPeriod: str):
    """
    :param schedulingStrategy:Could be CRON_DRIVEN or TIMER_DRIVEN
    :param schedulingPeriod: If CRON would be in the form of '* * * * * ?', else XX sec (or min)
    :return:
    """
    sche_strat = ['TIMER_DRIVEN', 'CRON_DRIVEN']
    if schedulingStrategy in sche_strat:
        if schedulingStrategy == 'TIMER_DRIVEN' and re.match('\d+\s(sec|min)$', schedulingPeriod):
            return True
        elif schedulingStrategy == "CRON_DRIVEN" and croniter.is_valid(schedulingPeriod[:-2]):
            return True
        else:
            return "Error: Wrong scheduling period"
    else:
        return "Error: Wrong scheduling strategy"

I am wondering how I can do if I have more scheduling strategy without multiplying if cases?

I had an idea of having a kind of dictionnary containing, the name of strategy and a validation function, is it the best way to do it?

Pdeuxa
  • 651
  • 7
  • 27
  • Returning a non empty string may be a bad idea because `if test_scheduling_parameters(...): code` will always enter the `if` block. – timgeb Nov 04 '20 at 10:38
  • Have you considered a switch block instead? – Serial Lazer Nov 04 '20 at 10:42
  • 2
    @SerialLazer a `switch` block? in python? – Chase Nov 04 '20 at 10:43
  • Python doesnt provide switch blocks out-of-the-box. But if you are after readability and maintainability, I highly recommend creating your own. Something like this: https://jaxenter.com/implement-switch-case-statement-python-138315.html – Serial Lazer Nov 04 '20 at 10:45
  • Does this answer your question? [Replacements for switch statement in Python?](https://stackoverflow.com/questions/60208/replacements-for-switch-statement-in-python) – MisterMiyagi Nov 04 '20 at 10:47
  • 1
    Are *all* your "scheduling strategies" tests of the form "name equality and verification function"? Is the same name ever used for multiple cases, separated only by the verification function? Note that the "Pythonic" way would be to have an opaque Strategy *object* selected by identity instead of name, which checks the period on instantiation and throws an error for invalid data. – MisterMiyagi Nov 04 '20 at 10:51

3 Answers3

1

Something like the below

def test_scheduling_parameters(schedulingStrategy: str, schedulingPeriod: str):
    """
    :param schedulingStrategy:Could be CRON_DRIVEN or TIMER_DRIVEN
    :param schedulingPeriod: If CRON would be in the form of '* * * * * ?', else XX sec (or min)
    :return:
    """
    sche_strat = ['TIMER_DRIVEN', 'CRON_DRIVEN']
    if schedulingStrategy == 'TIMER_DRIVEN' and re.match('\d+\s(sec|min)$', schedulingPeriod):
        return True, ''
    elif schedulingStrategy == 'CRON_DRIVEN' and croniter.is_valid(schedulingPeriod[:-2]):
        return True, ''
    else:
        return False, "Wrong scheduling period" if schedulingStrategy in sche_strat else 'Wrong scheduling strategy'
balderman
  • 22,927
  • 7
  • 34
  • 52
1

Using a dict of functions:

import sys
import re
import croniter

def checkstrat_timerdriven(schedulingPeriod):
  return re.match('\d+\s(sec|min)$', schedulingPeriod)

def checkstrat_crondriven(schedulingPeriod):
  return croniter.is_valid(schedulingPeriod[:-2])

def checkstrat_wrongstrat(schedulingPeriod):
  print("Error: Wrong scheduling strategy", file=sys.stderr)
  return False

def test_scheduling_parameters(schedulingStrategy: str, schedulingPeriod: str):
    """
    :param schedulingStrategy:Could be CRON_DRIVEN or TIMER_DRIVEN
    :param schedulingPeriod: If CRON would be in the form of '* * * * * ?', else XX sec (or min)
    :return:
    """
    checkstrat_dict = {'TIMER_DRIVEN': checkstrat_timerdriven, 'CRON_DRIVEN': checkstrat_crondriven}
    checkstrat = checkstrat_dict.get(schedulingStrategy, checkstrat_wrongstrat)
    return checkstrat(schedulingPeriod)
Stef
  • 13,242
  • 2
  • 17
  • 28
1

If number of lines is your only concern, have a predefined dictionary and keep your key as strategy_name and value as validator like below,

def test_scheduling_parameters(schedulingStrategy: str, schedulingPeriod: str):
    """
    :param schedulingStrategy:Could be CRON_DRIVEN or TIMER_DRIVEN
    :param schedulingPeriod: If CRON would be in the form of '* * * * * ?', else XX sec (or min)
    :return:
    """
    scheduling_validation = {
                                   'TIMER_DRIVEN': re.match('\d+\s(sec|min)$', schedulingPeriod),
                                   'CRON_DRIVEN': croniter.is_valid(schedulingPeriod[:-2])
                            }
    if schedulingStrategy in scheduling_validation .keys():
        if scheduling_validation[schedulingStrategy]:
            return True, ''
        else:
            return False, 'Wrong Scheduling Period'
    else:
        return False, 'Wrong Scheduling Strategy'
Maran Sowthri
  • 829
  • 6
  • 14