0

I am trying to write a function that validates a package that contains 2 dates (beginning and end), a destination and a price

Initially, I tried to write a function that "creates" dates and puts them in a list and then compares them to find out if the end date was lower than the beginning but I figured that was too complicated so I resorted to the datetime inbuilt module

However, when I try to run the test function, it fails and it outputs this error message

File "C:\Users\Anon\Desktop\Fac\FP\pythonProject\main.py", line 65, in valideaza_pachet
    raise Exception(err)
Exception: wrong dates!

I assume that I must have messed up a condition in the valideaza_pachet() function, but I don't understand what I did wrong

The code:

import time
import calendar
from datetime import date, datetime


def creeaza_pachet(data_i, data_s, dest, pret):
    # function that creates a tourism package, data_i = beginning date, data_s = end date, dest = destination and pret = price
    return {
        "data_i": data_i,
        "data_s": data_s,
        "dest": dest,
        "pret": pret
    }


def get_data_i(pachet):
    # functie that returns the beginning date of the package
    return pachet["data_i"]


def get_data_s(pachet):
    # functie that returns the end date of the package
    return pachet["data_s"]


def get_destinatie(pachet):
    # functie that returns the destination of the package
    return pachet["dest"]


def get_pret(pachet):
    # functie that returns the price of the package
    # input: pachet - un pachet
    # output: pretul float > 0 al pachetului
    return pachet["pret"]


def valideaza_pachet(pachet):
    #functie that validates if a package was correctly introduced or not
    #it raises an Exception as ex if any of the conditions aren't met
    err = ""
    if get_data_i(pachet) < get_data_s(pachet):
        err += "wrong dates!" # if the end date is lower than the beginning date
    if get_destinatie(pachet) == " ":
        err += "wrong destination!"
    if get_pret(pachet) <= 0:
        err += "wrong price!"
    if len(err) > 0:
        raise Exception(err)


def test_valideaza_pachet():
    pachet = creeaza_pachet(datetime.strptime('24/08/2021',"%d/%m/%Y"), datetime.strptime('26/08/2021',"%d/%m/%Y"), "Galati", 9000.1)
    valideaza_pachet(pachet)
    pachet_gresit = creeaza_pachet(datetime.strptime('24/08/2021',"%d/%m/%Y"), datetime.strptime('22/08/2021',"%d/%m/%Y"), "Galati", 9000.1)
    try:
        valideaza_pachet(pachet_gresit)
        assert False
    except Exception as ex:
        assert (str(ex) == "wrong dates!\n")
    alt_pachet = creeaza_pachet(datetime.strptime('24/08/2021',"%d/%m/%Y"), datetime.strptime('22/08/2021',"%d/%m/%Y"), " ", -904)
    try:
        valideaza_pachet(alt_pachet)
        assert False
    except Exception as ex:
        assert(str(ex) == "wrong dates!\nwrong destination!\nwrong price!\n")


def test_creeaza_pachet():
    data_i_string = '24/08/2021'
    data_i = datetime.strptime(data_i_string, "%d/%m/%Y")
    data_s_string = '26/08/2021'
    data_s = datetime.strptime(data_s_string, "%d/%m/%Y")
    dest = "Galati"
    pret = 9000.1
    pachet = creeaza_pachet(data_i,data_s,dest,pret)
    assert (get_data_i(pachet) == data_i)
    assert (get_data_s(pachet) == data_s)
    assert (get_destinatie(pachet) == dest)
    assert (abs(get_pret(pachet) - pret) < 0.0001)


def run_teste():
    test_creeaza_pachet()
    test_valideaza_pachet()


def run():
    pass


def main():
    run()
    run_teste()


main()
  • 1
    What did you expect *assert False* to do? Also, for something like this you will find a Python class very useful –  Oct 17 '21 at 12:27
  • When I wrote it my idea was that if I check pachet_gresit using the valideaza_pachet() function it would be False, since I wrote pachet_gresit as a wrong package, i.e. the end date is lower than the beginning date – manowar447233 Oct 17 '21 at 12:28
  • But *assert False* will raise an exception which you catch and report "Wrong dates" –  Oct 17 '21 at 12:29
  • Didn't get to read your edit, as for classes, we aren't allowed to use them yet (This is for a uni project if it didn't seem like it lol) – manowar447233 Oct 17 '21 at 12:29
  • True, but PyCharm outputs it as an error – manowar447233 Oct 17 '21 at 12:30
  • 1
    Consequential validation is a bad idea. Check the dates in *creeaza_pachet* and if the order is wrong, raise an exception –  Oct 17 '21 at 12:33
  • I already did that in * if get_data_i(pachet) < get_data_s(pachet): err += "date gresite!"* and if that is true, then the string err would get "wrong dates!" – manowar447233 Oct 17 '21 at 12:39
  • Take another look at your abuse of *assert False* in *test_valideaza_pachet*. Also, reconsider *get_destinatie(pachet) == " "*. That's almost certainly not what you want –  Oct 17 '21 at 12:42
  • What do you mean abuse of the assert function? I thought that if a package isn't entered properly, then it should be False. Also, the get_destinatie(pachet) == " " condition means that if the destination inputted is a blank space, then the program will output the error "wrong destination!" (it was in the task specifications) – manowar447233 Oct 17 '21 at 12:47
  • if `data_i = beginning date, data_s = end date` and beginning must be *before* end, then your comparison in `valideaza_pachet` is the wrong way, **it should be** `if get_data_i(pachet) >= get_data_s(pachet):` I think – FObersteiner Oct 17 '21 at 13:05
  • in general, I really have to support @BrutusForcus here, this whole package handling thing reads like a `class` would be a much better fit. Just an example, why would you create a `get` method that just returns the value of a dict? dictionaries already have a get method, so an on-top "getter" is basically useless, but makes much more sense in a class, see e.g. [here](https://docs.python.org/3/library/functions.html#property). – FObersteiner Oct 17 '21 at 13:09
  • yeah, it's true @MrFuppes, that was a mistake on my part, but I still get that assertion error at the ```valideaza_pachet(pachet_gresit)``` line – manowar447233 Oct 17 '21 at 13:12
  • again, I can't use classes, it was in the task specification – manowar447233 Oct 17 '21 at 13:13
  • Start by removing `assert False` - that just makes no sense. Then add meaningful messages to the assertions; e.g. `assert a==b, "a must be equal to b"` (see also [this question](https://stackoverflow.com/q/5142418/10197418)). That should give you a change to find where things go wrong. – FObersteiner Oct 17 '21 at 13:47
  • my mistake was when I added the error messages to the err string. I had forgotten to add the ```\n``` to the message so that's why my assertion failed :/ – manowar447233 Oct 17 '21 at 13:52

1 Answers1

1

This is more of a code review and kind of off-topic here but... First,

  • drop all assert False - these won't do anything useful
  • drop the getter functions, that just makes things convoluted (just use dict[key] in the code as long as you don't use custom class)
  • drop unnecessary imports like calendar
  • you might also want to drop the run and main functions (again convoluted) - just call the functions you need

Then you could change valideaza_pachet to raise specific value errors:

def valideaza_pachet(pachet):
    if pachet["data_i"] >= pachet["data_s"]:
        raise ValueError("start date must be < end date")
    if pachet["dest"] == " ":
        raise ValueError("destination must not be empty")
    if pachet["pret"] <= 0:
        raise ValueError("price must be >= 0")
    # returns None if no exception was raised

Now to test a valid package, you would just do

valideaza_pachet(pachet) # no exception expected

Testing an invalid package is a bit more complicated without a class that can be unit-tested (see e.g. here) - but you can catch the exception and use the else clause of the try/except to raise an AssertionError that says you wanted an exception:

try:
    valideaza_pachet(pachet_gresit)
except Exception as ex:
    print(f"successfully received exception: {ex}")
else:
    raise AssertionError("validation should have raised an Exception")

or even

assert str(ex) == "start date must be < end date", f"got '{ex}', expected 'start date must be < end date'"

in place of the print statement.

FObersteiner
  • 22,500
  • 8
  • 42
  • 72
  • thanks for the tips, didn't actually know you could call dict[key] instead of writing the get_ functions, our lecturer only showed us the latter way :/ as for classes, I said twice that I can't use them, but I'll do more research on them anyways – manowar447233 Oct 17 '21 at 14:00
  • @manowar447233 no worries, that application is just so well-suited for a `class` ^^ Maybe the lecturer is trying to prepare for that. Hope I could give you some insights. Btw. dictionaries also have a [get](https://docs.python.org/3/library/stdtypes.html#dict.get) method which is helpful if you don't know if the key you want is in there - if not, it doesn't raise a KeyError but returns a default instead. – FObersteiner Oct 17 '21 at 14:04