Currency Exchange exercise -- suggested naming tweaks

Programming Python
1

Hello,

I just finished the currency exchange exercise. The exercise itself is straight forward, and the explanations are excellent (seriously!), but I managed to confuse myself, and I spent a bit of time trying to understand why.

It was only after I got the last task wrong that I realized that I had spent the entire exercise assuming that the number_of_bills thing had to do with the original currency.

In retrospect (a) this is a totally silly assumption, and (b) the instructions clearly state that The total you receive must be divisible by the value of one “bill” or unit. (Emphasis added)

However, I was paying a lot more attention to the code than the instructions, and in the code it says:

def get_number_of_bills(budget, denomination):
    """

    :param budget: float - the amount of money you are planning to exchange.
    :param denomination: int - the value of a single bill.
    :return: int - number of bills after exchanging all your money.
    """

    pass

The key bit that I think led me down the garden path was the name budget and the description the amount of money you are planning to exchange. To me budget definitely has the flavor of the amount of money that I’m starting out with, rather than the amount of money that I receive after making the exchange.

I can think of two ways to tweak this.

  1. Make sure the two methods are clearly talking about what you have/receive after the exchange.
  2. Go more generic, and not talk about which side of the exchange it’s on (since technically it works for any currency). So instead of budget, maybe amount or funds or total_value.

Here’s an example of what “more generic” could look like:

def get_value_of_bills(denomination, number_of_bills):
    """

    :param denomination: int - the value of a bill.
    :param number_of_bills: int - number of bills of that denomination.
    :return: int - total value of the bills.
    """

    pass

def get_number_of_bills(amount, denomination):
    """

    :param amount: float - the total amount of money available.
    :param denomination: int - the value of a single bill.
    :return: int - number of bills that can be obtained with the available funds.
    """

    pass

The second thing that I was confused about was the exchangeable_value(). I assumed that exchangeable meant the amount that could be exchanged. In other words, I thought this was about the original currency rather than the target currency. After reading through the task description a few times I did figure it out, but I wonder if there’s a name that would help make it clearer that it’s about the target currency after the exchange. I can’t think of anything great off the top of my head, but maybe something like obtainable_value?

I’d love to hear your thoughts.

If, after discussing this the community does come up with better names, I’d be happy to submit a pull request adjusting the exercise.

1 Like
2

Hi @MangoSeed :wave: welcome to the Exercism forums!

Thank you for your comments on Currency Exchange - this exercise has been a challenging one to develop for the Python track. We’ve even considered deprecating it in favor of another numbers-based challenge.

It has the lowest completion rate of any concept exercise after Guido's Gorgeous Lasagna, and I suspect that the wording of the exercise and explanations has been a big part of that. Many contributors have worked very hard to clarify this exercise and improve the instructions – and we clearly have room for more improvements. :slightly_smiling_face:

Hard agree here. I think budget may have been chosen for some sort of consistency, but is highly confusing for these tasks, given the parameters and instructions on task 6.

I like your option 2: go more generic with the parameter naming and docstring (I still am mulling over whether or not we go more generic in the instruction text, and leaning toward being more specific there). Since the parameters can be named anything and not change the requirements, its not going to have an impact on already existing student solutions.

I might even go even more generic:

def get_value_of_bills(denomination, number_of_bills):
    """

    :param denomination: int - the value of a bill.
    :param number_of_bills: int - total number of bills.
    :return: int - calculated value of the bills.
    """

    pass

def get_number_of_bills(amount, denomination):
    """

    :param amount: float - the total starting value.
    :param denomination: int - the value of a single bill.
    :return: int - number of bills that can be obtained from the amount.
    """

    pass

I think it would then follow that we would want to also rename budget in task 5:

def get_leftover_of_bills(amount, denomination):
    """

    :param amount: float - the total starting value.
    :param denomination: int - the value of a single bill.
    :return: float - the amount that is "leftover", given the current denomination.
    """

    pass

Just so we have the stub for the task being discussed:


def exchangeable_value(budget, exchange_rate, spread, denomination):
    """

    :param budget: float - the amount of your money you are planning to exchange.
    :param exchange_rate: float - the unit value of the foreign currency.
    :param spread: int - percentage that is taken as an exchange fee.
    :param denomination: int - the value of a single bill.
    :return: int - maximum value you can get.
    """

    pass

I think the parameters are correctly named. budget is indeed referring to the amount of money to be exchanged, and the function does indeed calculate what value you would get back from that exchange - effectively the “exchangeable value” (you might have $127.50 in your budget, but you aren’t going to get more than an €80.00 value from that sleazy exchange booth). With the other parameter renames, perhaps it becomes clearer what this function is expected to do?

My concern here is that changing the function name would invalidate roughly 26, 883 passing student solutions. That is fine if we truly need to do that.

But I am not sure that swapping obtainable for exchangeable in the function name is an important enough clarification (both read equally obscure to me). Is there anything we can do with the instructions or examples that might help clarify this, or a different way of phrasing the docstring that could help?

@IsaacG (because you mentor this a lot) , @vaeng (because you’ve helped with wording here in the past), @kytrinyx (as a follow-on to your last PR) – your thoughts would be most welcome here. :slightly_smiling_face:

The TL;DR:

I would welcome a PR for the parameters (and the associated text changes in the introduction.md and the instructions.md) – but need more discussion/convincing for the function name change.

3

I like the idea of making the parameter names more generic (amount). I think that makes the function more clear and demonstrates how you’d write a “generic” exchange-oriented function to solve a specific use case (having a budget).

I also agree that renaming the function is a big ask when it’s not obvious that a new name would make a significant difference. Would it make sense to instead change the function’s docstring to try to explain what it is supposed to be doing?

4

Thank you @BethanyG – I like your suggestion of going even more generic. That seems very clean and understandable.

I will submit a PR making the suggested changes.

On the second one, I agree that obtainable isn’t great, and especially in light of invalidating several thousand solutions, is definitely not worth doing.

Thanks so much for taking the time to read and talk through this.

5

I opened the PR here: Clarify bills in currency exchange exercise by LittleMangoSeed · Pull Request #3520 · exercism/python · GitHub

6

I’ve just been notified that my post was flagged as spam by the community.

It says:

Your post was flagged as spam : the community feels it is an advertisement, something that is overly promotional in nature instead of being useful or relevant to the topic as expected.

Would someone please help me understand what I did wrong?

Another post was flagged at more or less the same time. Did I offend a particular community member? Or did I make some bot mad?

7

That is very strange. It must have been by mistake! @iHiD - I can neither see the hidden post, nor did I (that I know of) mark it as spam.

Could you take a look?

8

Ok, thanks! If I do misstep at any point, please do let me know.

9

I was really excited to use this platform and then I get to the second exercise and the inconsistencies in the variables defined in the instructions drive me crazy

Example:
budget (float): Tthe amount of money you are planning to exchange.
… this tells me it’s a portion of the total money
budget (float): The amount of money you own.
…this tells me it’s all the money you own
float: The amount left of your starting currency after the exchange
…suggests that you’re only exchanging a portion of budget
This function calcultes and returns the amount of money left over from the budget after an exchange.
…again suggests that only a portion of budget is exchanged.

When budget is the total of what you own and what you’re planning on exchanging., some of the later tasks don’t make sense. Later on it appears you subtract converted currency from budget, but you can’t because budget and exchange_value are different units. Maybe I missed something but budget - exchange_value doesn’t make sense.

10

The budget is the maximum amount of money you can use for the operation. Whether or not you own more money than that really doesn’t matter. You’ll never use more money than the budget for any given operation.