Costs of characters in articles

Question

This code is supposed to read two files and calculate the potential cost of all its characters. The file with costs (lettercosts.txt) looks like this:

a 1
b 3
c 2 etc...

Here I was trying to make this all work, but I so far it is all unsuccessfully. Any hint where is the problem with the code?

def generate_cost_dict():
    letter_costs_dict = {}
    file = open("lettercosts.txt")
    with open("lettercosts.txt") as file:
       letter_cost_dict = {letter: int(cost)
                  for letter, cost in map(str.split, file)}
    return letter_costs_dict

def calculate_cost(article, lc_dict):

    with open("news1.txt") as x:
        for char in x.read():
            return sum(letter_cost_dict.get(char, 0)
    with open("news2.txt") as y:
        for char in y.read():
            return sum(letter_cost_dict.get(char, 0)              

def main():
    # Generating the mapping from letters to their USD costs
    lc_dict = generate_cost_dict()

    # Calculating the costs of the sample articles
    x = calculate_cost( "news1.txt", lc_dict )
    y = calculate_cost( "news2.txt", lc_dict )

    print("news1.txt costs",x,"USD.")
    print("news2.txt costs",y,"USD.")


if __name__ == '__main__':
    main()

the function `calculate_cost` will never go further than the first letter of the first file, since you use return inside the for loop — Julien Spronck, May 17 '15 at 05:38
the arguments `article` and `lc_dict` of `calculate_cost` are not used. This should result in an error as `letter_cost_dict` is undefined. Do you get an error? — Julien Spronck, May 17 '15 at 05:42
another couple remarks: 1) you `open` the "lettercosts.txt" file twice => remove the first open line; 2) `file` is a built-in type in Python, don't use that word as a variable name — Julien Spronck, May 17 '15 at 05:47
another problem: `generate_cost_dict` will return an empty dict since you return `letter_costs_dict` instead of `letter_cost_dict` — Julien Spronck, May 17 '15 at 05:53
`return sum(letter_cost_dict.get(char,0)` is missing a close paren, among the other multiple mistakes. — Adam Smith, May 17 '15 at 06:19
Someone should edit this question and revert to the older version which had the original code and question. OP, or someone, has edited it out. (I'm on the app, so can't revert.) In its current form, the question makes no sense and is totally disconnected from the answer by @AdamSmith. — aneroid, May 17 '15 at 16:35

score 1 · Answer 1 · edited May 17 '15 at 06:39

1

The minimal changeset to your code to make it run looks like:

def generate_cost_dict():
    letter_costs_dict = {}
    file = open("lettercosts.txt")
    with open("lettercosts.txt") as file:
       letter_cost_dict = {letter: int(cost)
                  for letter, cost in map(str.split, file)}
    return letter_costs_dict

def calculate_cost(article, lc_dict):

    with open(article) as x:
        accum = 0
        for char in x.read():
            accum += lc_dict.get(char, 0)
        return accum           

def main():
    # Generating the mapping from letters to their USD costs
    lc_dict = generate_cost_dict()

    # Calculating the costs of the sample articles
    x = calculate_cost( "news1.txt", lc_dict )
    y = calculate_cost( "news2.txt", lc_dict )

    print("news1.txt costs",x,"USD.")
    print("news2.txt costs",y,"USD.")


if __name__ == '__main__':
    main()

Though the minimal code is likely:

with open('lettercosts.txt') as f:
    LC_DICT = {lett:int(cost) for line in f for lett,cost in [line.split()]}

def calculate_cost(article):
    with open(article) as f:
        return sum(LC_DICT.get(ch, 0) for line in f for ch in line)

x_cost = calculate_cost("news1.txt")
y_cost = calculate_cost("news2.txt")

edited May 17 '15 at 06:39

aneroid

12,983
3
36
66

answered May 17 '15 at 06:23

Adam Smith

52,157
12
73
112

@aneroid thanks, and also thanks for fixing my typo. Decided to use `lc_dict` as a constant after writing the code and only changed it where it was assigned :) – Adam Smith May 17 '15 at 06:43
@aneroid: do you mean the last code fragment (if we remove globals and provide more descriptive names)? Because the first one has dead code in it. – jfs May 17 '15 at 16:21
@J.F.Sebastian Yes, the last one in AdamSmith's answer. The first one is based on the OP's code (which he/she has edited out), with minimal changes to make it work. And no, the OP's code was not good. I think that's why the correct concise version. Easier for the OP to learn/figure out that way. – aneroid May 17 '15 at 16:29
It reminds of the quote attributed to Ken Thompson: [*“I'd spell create with an 'e'.”*](http://unix.stackexchange.com/q/10893/1321). As a rule of thumb: your code should pass a spell-checker: `LC_DICT -> char_cost`, `lett -> char`, `f -> file`, `ch -> char`, `x_cost -> news1_total_cost`. – jfs May 17 '15 at 16:48
@J.F.Sebastian agreed with `char_cost` and `news1_total_cost`, but `lett` and `ch` are perfectly understandable (and `char` might pass a spell check but unless you're burning the word it doesn't mean what the dictionary thinks it means), and `f -> file` shadows a builtin. Do you name all your temp strings `str` too? :) – Adam Smith May 17 '15 at 18:44
@AdamSmith: `file` is in a different category than `str`: [you should *almost* always use `open()`](http://stackoverflow.com/q/11942482/4279) -- [A Foolish Consistency is the Hobgoblin of Little Minds](https://www.python.org/dev/peps/pep-0008/) – jfs May 17 '15 at 19:03

Costs of characters in articles

1 Answers1