Updated:

# ORDER TYPE COUPONS
f'10% off order' : lambda x: x // 10,
f'10$ off 20$ or larger order' : lambda x: 1000 if x >= 2000 else 0,

# Reward points is a special order type coupon that gives $1 discount per 1000 points.
f'Reward points: {0 <= n <= 100000}' : lambda x: 100 * x // 1000

order type coupons should each be processed exactly once if they exist.

Let me know if that's a clear enough description for the rewards coupons.

It matters from the standpoint of de-duplicating coupons: de-duplicating by the entire coupon name rather than reward coupons will fail tests where multiple reward coupons of different values exist. This isn't ruled out by the description, so there are no reasons to arbitrarily remove some of the possible inputs.

I see your point now. My appologies for not understanding.

I have changed tests to use strict equals.

Hello mortonfox, The discrepancy is due to assert_approx_equals accidentally being left in after converting all the floats to use ints. Let me know if this solves your problem.

I chose the puzzles tag because I don't know what other category this falls under. I figured a vague over-used tag would be better than no tag. If there's one that's more applicable, or if no tag is actually better, I'd be happy to change it.

I understand your frustration, but yes. To the best of my ability, I have tested that this works the way it was intended. I see the issue you're describing now, and I'm not sure if it matters. If given a coupon array of one reward points coupon, you'll get a random number that should be given to the coupon for the discount amount. If you recieve two or more rewards coupons, while, yes they all have the same number currently, it doesn't matter. That coupon should only be processed one time anyway. If it's that important to add the appearance of randomness, I will happily do this. I just don't see the reason given that the rules were pretty explicit about how to process duplicate coupons.

Fixed!

This has been fixed. Those lambda functions now require two variables, but they have been adjusted to fix the anti-pattern. I'll do some reading on that link you provided to make sure everything else is working right.

I have changed the kata to use only integers.

This description is just plain wrong. In fact these are 3 different kinds of coupons, and the accompanied functions all have different signatures.

I agree with your observation about how the coupons function. I've double checked that the logic works correctly with this usage, and it can be correctly calculated as described, as long as you consider how and why these functions return the number that they do. However, I've changed the descriptions of coupons drastically to account for some of the other issues raised. This specific wording is no longer in the description.

Reference solution is inconsistent over itself on this matter too.

I believe this is incorrect. n% off order and n% off x$ or larger order do not exist in the actual testing. Much like actual coupons, these two discounts are predetermined and will not change. The random coupons cover a pretty wide variety of items, which is the main contributor to the total. I didn't really see the need to go too overboard with sub-total manipulation type coupons since this value is fixed after items are scanned.

Related issue: reward points coupons are always the same inside each test. This is defintely unexpected.

I really hate saying this, but I cannot replicate the issue. I added a print statment each time a rewards coupon is generated, and then ran the kata about 10 times. I copied the logs into a pretty simple parser that isolated the numbers, and I couldn't find any that were duplicated. I'm using import random for the random tests, and I'm not changing the seed anywhere, so it should be automatically updated, right? If you can replicate, or have something I could try, just let me know and I'll jump on it asap.

Some coupons are incorrectly implemented.

10% off order was fixed. 2 for $5 can be correctly implimented using the modified lambda. If I need to update the description to indicate that the lambda functions are not all encompasing, but simply one option to calculate the correct discount, let me know. From the current description:

All coupons should be based on the sub-total and NOT the final total. All coupons come with a lambda function that you are free to use, but not required to. Either way, it is up to you to apply the coupon correctly. (For those unaware, BOGO is short for buy one, get one free.) See example:

{
# ITEM TYPE COUPONS
f'BOGO {item}' : lambda x: x // 2,
f'2 for $5 {item}' : lambda x: 5 * (x // 2),
f'25% off {item}' : lambda x: x * 0.25,

# ORDER TYPE COUPONS
f'10% off order' : lambda x: x * 0.1,
f'10$ off 20$ or larger order' : lambda x: 10 if x >= 20 else 0,
f'Reward points: {0 <= n <= 100000}' : lambda x: x // 1000
}

Reference solution fails to handle coupons that applies to an item with space in its name.

Fixed. Simple typo... thanks for your keen eye!

Reference solution fails to handle the case where the first applied coupon results in no discount (when the coupon's either BOGO or 2 for $5, and only 1 item is bought), which will then apply the second coupon.

Fixed. Logic for this check has been removed from the lambda function and placed at the beginning of coupon evaluation.

Fixed. Updated the description to clarify that the first occurrence should be applied.

Kata design issue: when payment is done in cash, it is expected the change to be returned. When payment is done via bank only ACCEPTED needs to be returned; but the cost are not deducted from the bank? So we're not actually paying for the products? This is very unrealistic.

Thanks for this feedback. I want to ask about why this is unrealistic though. In my experience, a self checkout machine is only responsible for soliciting sales of items for the store. If cash is given, any change owed is returned to the customer. If a card is given, the card reader checks with the bank if the funds are available, and then the machine will send an invoice to the bank to withdraw the money. From the perspective of an average user buying groceries, I think this is a more accurate system. While yes, there are more things that happen on the backend with the actual account, this seems unrelated. If it were to be fully implemented, the only safe way to do it (since BANK is in preloaded right now) would be to ask warriors to implement a seperate function to manage this extra logic. With other kata covering this in spades, it seems like an unnecessary way to bloat the logic of what we're trying to do here.

preloaded.COUPONS is completely useless: it's a string of newline-delimited key-values which is a hassle to parse, and this is not needed anyway since the coupons pass in are already the parsed form of the key-value pairs.

I have removed this. I'm unsure about how parsing works, but when I was testing this myself, I could only manage to see the memory key that the lambda was held in. If this is something that is easy to print, or easy to test, that's all that matters to me. I just didn't want to pass the user a cryptic function and say "eh, figure it out".

2 for $5 coupons for the majority of the items are actually pointless because most items are priced below 2.5, in which case buying 2 for $5 would be paying more than the normal price.

I will add a check to the coupon randomizer to account for the price of an item.

Fixed.

@mortonfox Thanks for the heads up. I've upded the tests to use 0.001 accuracy with approx_equals. I'm pretty sure your solution will work now. Sorry for the trouble.

@hobovsky and @akar-0 Thanks for the links. I've thought about having the tests use rounding to 2 decimal places (since we're representing money after all), but since there are issues with floats generally, I didn't want to make the requirements to difficult for rounding errors. I've never used the decimal module, but I'd be happy to do some digging and implement that into the tests if you think that would make things better and more consistent?

alternatively, I could change the way we're calculating sales tax, since (at least in my code) that's the only place a float is used. I could change it to use max(1, subtotal // 10)) which would be able to return a clean integer for tax, but then we get into the potential for rounding errors.

Any direction is appreciated. I'll wait to resolve the issue until I either come up with something, or we decide how it should be changed.

It looks good except for the way it reports fails on a random test. The HTML doesn't seem to work. Should give a useful message instead. I would reccomend deleting all HTML font coloring and stuff, and change it to return something like:

Testing for:\n
{string being tested}

After that, the standard answer should equal expected would be fine.

I'd be happy to make a fork with these updated if you've got something else on your hands. Just let me know!