I'm learning Python by solving previous GCJ problems. This problem is quite simple to solve, but I'd like my code to be reviewed for advance (more Pythonic expression and more efficient algorithm).

The magician starts by arranging 16 cards in a square grid: 4 rows of cards, with 4 cards in each row. Each card has a different number from 1 to 16 written on the side that is showing. Next, the magician asks a volunteer to choose a card, and to tell him which row that card is in.

Finally, the magician arranges the 16 cards in a square grid again, possibly in a different order. Once again, he asks the volunteer which row her card is in. With only the answers to these two questions, the magician then correctly determines which card the volunteer chose. Amazing, right?

You decide to write a program to help you understand the magician's technique. The program will be given the two arrangements of the cards, and the volunteer's answers to the two questions: the row number of the selected card in the first arrangement, and the row number of the selected card in the second arrangement. The rows are numbered 1 to 4 from top to bottom.

Your program should determine which card the volunteer chose; or if there is more than one card the volunteer might have chosen (the magician did a bad job); or if there's no card consistent with the volunteer's answers (the volunteer cheated).


The first line of the input gives the number of test cases, T. T test cases follow. Each test case starts with a line containing an integer: the answer to the first question. The next 4 lines represent the first arrangement of the cards: each contains 4 integers, separated by a single space. The next line contains the answer to the second question, and the following four lines contain the second arrangement in the same format.


For each test case, output one line containing "Case #x: y", where x is the test case number (starting from 1).

If there is a single card the volunteer could have chosen, y should be the number on the card. If there are multiple cards the volunteer could have chosen, y should be "Bad magician!", without the quotes. If there are no cards consistent with the volunteer's answers, y should be "Volunteer cheated!", without the quotes. The text needs to be exactly right, so consider copying/pasting it from here.



1 2 3 4
5 6 7 8
9 10 11 12
13 14 15 16
1 2 5 4
3 11 6 15
9 10 7 12
13 14 8 16
1 2 3 4
5 6 7 8
9 10 11 12
13 14 15 16
1 2 3 4
5 6 7 8
9 10 11 12
13 14 15 16
1 2 3 4
5 6 7 8
9 10 11 12
13 14 15 16
1 2 3 4
5 6 7 8
9 10 11 12
13 14 15 16


Case #1: 7
Case #2: Bad magician!
Case #3: Volunteer cheated!

And here's my code:

import sys
from itertools import repeat

def main():
    submit = False
    filename = "A-small-practice"

    sys.stdin = open(filename + ".in")
    if submit:
        sys.stdout = open(filename + ".out", "w")

    test_cases = int(input())

def main2(test_cases):
    for cn in range(1, test_cases + 1):
        print "Case #" + str(cn) + ":",
        choices = {}

        # Each test case has 2 answers.
        for i in repeat(None, 2):
            # Volunteer's answer (row # which contains his choice)
            row = int(input())

            # Cards are in 4 rows.
            for r in range(1, 5):
                nums = raw_input()
                if r == row:
                    # A row has 4 cards.
                    for n in nums.split():
                        choices[n] = choices.get(n, 0) + 1

        # The chosen card(s) would be appeared twice.
        # So, pick items in choices whose value(appearance) is 2, and filter
        # others.
        choices = {k: v for k, v in choices.iteritems() if v == 2}

        # Only one choice should exist after filtering.
        if len(choices) == 1:
            print choices.keys()[0]
        elif len(choices) > 1:
            print "Bad magician!"
            print "Volunteer cheated!"


Any comments are welcome.


2 Answers 2

  • Naming: main is reserved so that's ok, but main2 makes no sense. The name should be a hint about what the function does, I'd go for guess_card.
  • Since there's no mention of a file input, I'd say go for standard input. You can always read a file and redirect the input if that's needed. The same goes for standard output.
  • You're using both input and raw_input, why is that? If you want to do input validation, you should do it properly, trying to parse it and eventually catching exceptions. But if you go down that road, then you should also not assume that the rest of the input is correct. I'd say that for this specific program, you can safely say that the syntax of the input will be fine.
  • Comments used like this are pretty much useless, they should tell why things are happening the way they are, not re-describe the code
  • Your second function is not modular, it will take all cases and do everything. I think you should make that function guess one of the cases and loop around it instead of inside.
  • There's really no need to do anything differently from what the "magician" is doing, no need to import itertools, just keep in mind that user answers are a 1-based index, while arrays are 0-based.

Here's my take on it:

def guess_card(user_rows, card_rows):
    possible_answers = []
    possible_cards = card_rows[user_rows[0] - 1]

    for card in possible_cards:
        if card in card_rows[user_rows[1] + 3]:
            if len(possible_answers) > 1:
                return 'Bad magician!'

    if len(possible_answers) == 0:
        return 'Volunteer cheated!'

    return possible_answers[0]

def main():
    n_test_cases = int(raw_input())
    answers = []

    while n_test_cases:
        n_test_cases -= 1
        user_rows = []
        card_rows = []
        for i in range(0, 4):
            card_rows.append(raw_input().split(' '))

        for i in range(4, 8):
            card_rows.append(raw_input().split(' '))

        answers.append(guess_card(user_rows, card_rows))

    for answer in answers:
        print answer

if __name__ == "__main__":

If you're wondering about the if __name__ == "__main__": line, you can have a look here.

  • \$\begingroup\$ In guess_card you could calculate card_rows[user_rows[1] + 3] at the beginning of the function (and possibly make it a set. In main you could use for _ ni range(n_test_cases) instead of the while loop. \$\endgroup\$
    – Graipher
    Commented Feb 3, 2017 at 11:45

Use with for file objects

And generally anything that connects with the outside world. with statements automatically handle clean-up of these pesky objects at the end of the block.

Use list and dictionary objects where possible

Most of the built-in Python functions apply to these objects. Using them allows you to employ more of the handy Python built-ins.

On that note:

Some points about Python-specific techniques:

  • An input file (as stated in the assignment) is easily converted to a list using a list comprehension.

As in:

    file_lines = [i[:-1] for i in open("input.txt")]

You can also use this alongside the with function. Turning the file handler object immediately into a list means you can start using those amazing Python abilities ASAP.

  • Use set objects to find matches

The magician's trick works by finding cards that are common to both rows, before and after moving the cards. Let's call them a and b. Once you have both rows as sets, it's easy to find the common cards:

  • use slices and split to handle string input data.

Good work for doing this eg. in for n in nums.split. In fact, most of this assignment is simple string manipulation, which doesn't really require a lot of dressing-up in your code.

I've added my short, simple take on the assignment below:

message = ["Bad magician!",0] + ["Volunteer cheated!"]*3

file_lines = [i[:-1] for i in open("input.txt")]

for i in range(int(file_lines[0])):
    offset = (i * 10) + 1
    a = set(file_lines[offset + 0 + int(file_lines[offset + 0])].split(" "))
    b = set(file_lines[offset + 5 + int(file_lines[offset + 5])].split(" "))
    message[1] = a.intersection(b)
    print("Case #" + str(i + 1) + ": " + str(message[len(message[1])]))

(Note: It's likely an object-oriented version would be most re-useable and adabtable to other projects. If you'd like ideas about that ask me or anyone else here.)

  • \$\begingroup\$ Thanks for the tips. BTW the output of your code shows a guess number as Case #1: set(['7']), not Case #1: 7, so I edited a little. Can I edit this code more simpler? \$\endgroup\$
    – philipjkim
    Commented Feb 7, 2017 at 5:31
  • 1
    \$\begingroup\$ You can add [0] after msg[len(msg[1])] in the last line, and add an extra set of square brackets around "Bad magician!" and "Volunteer cheated!". This makes it so the output will be sliced properly regardless of whether it is the card number or the text. It works, but it's not exactly pretty. \$\endgroup\$
    – Ryan Mills
    Commented Feb 7, 2017 at 6:23
  • \$\begingroup\$ I'm sure there's a way to do this exercise and make it short AND sweet. We're not there yet. \$\endgroup\$
    – Ryan Mills
    Commented Feb 7, 2017 at 6:24

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.