New to Python and recently turned in an assignment that, in the grading, was reviewed as 'very bad' and 'doesn't use sets' with no further elaboration. Wondering what I can do to improve this code.

The assignment instructions and restrictions were as follows:

Problem : Card Collections (cards.py):

In class we talked about collecting packs of cards which form a set. For the purposes of this problem, the full set of cards is numbered 0 to n-1. In addition, each card has a value, in cents. Cards may have different values. Each pack of cards has k cards, some of which may be duplicates. Since we only care about new cards, we immediately give all duplicates away.

Write a program that reads in from the user the number of baseball cards in the set (n), the values of each of those n cards, and the contents of three packs of cards.

Your program should make the following computations:

(1) Calculate the total value of all the unique cards in the three packs all together.

(2) Determine which two packs of the three, taken together, create the most value, and print out that value. Implementation Restrictions In order to earn full credit, you must store each pack as its own set.

Furthermore, you must write a function that takes in both a set of cards and a full list of the values of all cards, and returns the total value of the set. Here is the function protocol:

Pre-condition: setOfCards is a set of integers in between 0 and n-1, where n is the length of the list listOfCardValues, and listOfCardValues[i] is the value of card number i.

Post-condition: Returns the total value of the cards specified by the set setOfCards. def totalValue(setOfCards, listOfCardValues):

And here is my submission(please be critical but explain the issues in my code as I am looking to improve as much as possible):

# Note: I have some testing print statements in the code if you want to activate them to check out the set structure.
# I made significant structure changes. First, I dedicate the data structuring to the first function, cardData()
# Additionally, I had cardData() return a list of lists to leave the totalValue() parameters the same as in the prompt.
# Another adjustment I made was in the way I called cardData() and other variables to eliminate code bloat.

def main():

    # This function takes in card data and returns setOfCards list and listOfCardValues list.
    def cardData():

       # These are the first two sets we will build so I've initialized them here.
        setOfCards = []
        listOfCardValues = []

        # This is a grab for the top value of our loop.
        total_cards_in_set = int(input("How many total cards in the set?\n> "))

        # Now we ask the real question, using the stop value from the previous question.
        print("List the value, in cents, of each card.\n")

        # This is the starting value for the 'set_of_cards' indexing set.
        index = 0

        # Setting only the stop value ensures it is n-1.
        for card in range(total_cards_in_set):
            value = int(input("> "))
            # appends the current card no. to the setOfCards set.
            index += 1
        # This shows specifically what cardData() yields before the return of list of lists.
        # print(setOfCards, listOfCardValues)
        return [setOfCards, listOfCardValues]

    # This function takes in 2 parameter lists and returns user values.
    def totalValue(setOfCards, listOfCardValues):

        #    print(f"The card index set: {setOfCards}")
        #    print(f"The card value set: {listOfCardValues}")

        # Here we initialize the sets that contain the values of the respective cards.
        pack1Cards = []
        pack2Cards = []
        pack3Cards = []

        # This variable gives the stop for each sub loop when we are filling each pack#_cards set.
        cards_in_pack = int(input("How many cards in a pack?\n> "))

        for packNum in range(0, 3):
            print(f"Input the card values in pack {packNum + 1}")
            if packNum == 0:
                for cardVal in range(0, cards_in_pack):
                    pack1Input = int(input("> "))
            elif packNum == 1:
                for cardVal in range(0, cards_in_pack):
                    pack2Input = int(input("> "))
                for cardVal in range(0, cards_in_pack):
                    pack3Input = int(input("> "))

        #    print(f"The card values in set 1: {pack1_cards}")
        #    print(f"The card values in set 2: {pack2_cards}")
        #    print(f"The card values in set 3: {pack3_cards}")

        # We init a dict for all the sums that we're going to play with.
        packValues = {
            'sum1': sum(pack1Cards),
            'sum2': sum(pack2Cards),
            'sum3': sum(pack3Cards),

        # Initialize the output sets.
        bestValueNamed = []
        bestValue = []

        # Check for order of magnitude then append as the sudoku puzzle fills itself.
        if packValues['sum1'] > packValues['sum2'] or packValues['sum1'] > packValues['sum3']:
            bestValueNamed.append('pack 1')
            # Must nest if statement so that best_value_named set will be ordinal, else output could be 'pack 3 and pack 1'.
            if packValues['sum2'] > packValues['sum3']:
                bestValueNamed.append('pack 2')
                bestValueNamed.append('pack 3')
            bestValueNamed.extend(['pack 2', 'pack 3'])
            bestValue.extend([packValues['sum2'], packValues['sum3']])

        #    print(f"Sum of pack 1: {pack_values['sum1']}")
        #    print(f"Sum of pack 2: {pack_values['sum2']}")
        #    print(f"Sum of pack 3: {pack_values['sum3']}")

        print(f"The total value of all three packs is {sum(listOfCardValues)}.")
        print(f"The two packs with most value are {bestValueNamed[0]} and {bestValueNamed[1]} "
              f"worth {sum(bestValue)} cents.")

    tempArray = cardData()
    setOfCards = tempArray[0]
    listOfCardValues = tempArray[1]
    # print(tempArray)
    # this array will show the list of lists used to scale totalValue parameters.
    totalValue(setOfCards, listOfCardValues)


1 Answer 1


Just right off the bat, you describe lists about your cards as sets.

Lists are different than sets. That's why your teacher is saying you didn't use sets. Sets are created with {} curly brackets, not [] square brackets. What's what is, of course, asked & answered on Stack Overflow.

  • 1
    \$\begingroup\$ Perfect, thanks! Found a good resource that explained the difference. \$\endgroup\$
    – Jcode94
    Commented Apr 10, 2020 at 19:00
  • 2
    \$\begingroup\$ This answer isn't quite correct; tuples are created with parentheses ('a', 'b', 'c'), while sets are created with curly braces {'a', 'b', 'c'}. However, this is rare to see (in my experience) as they're normally associated with dicts {'a':1, 'b':2}. Therefore it's normally clearest to specify the type foo = set(), passing an iterable (perhaps a tuple) as the first arg or adding values as you go. \$\endgroup\$
    – ti7
    Commented Apr 11, 2020 at 4:48
  • 1
    \$\begingroup\$ Oh hey that’s totally right. Sleepy brain. Editing, thanks. \$\endgroup\$
    – Preston
    Commented Apr 11, 2020 at 4:49
  • 1
    \$\begingroup\$ @Jcode94 could you add a link to the resource here? It could be of use to others! \$\endgroup\$
    – ti7
    Commented Apr 11, 2020 at 4:49
  • 2
    \$\begingroup\$ One problem is that 'set' has two different meanings. In programming and math it refers to a specific kind of collection in which there are no duplicates. In collectable card trading it can refer to a specific group of cards which were printed together. For example, in the early days of Magic the Gathering, there were multiple sets, each of which had different cards. Examples might be Unlimited, Arabian Nights, Artifacts, etc. If the teacher had not been teachings sets before this assignment, I might have misunderstood. \$\endgroup\$
    – NomadMaker
    Commented Apr 11, 2020 at 10:54

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.