patternpythonMinor
Unique letter counter for string in Python
Viewed 0 times
uniqueforpythonlettercounterstring
Problem
I'm trying to create a program that creates a record of how many times letters appear in a given string (ignoring blank spaces) using recursion.
Is the global variable necessary? Should I be using a different data structure instead of a dictionary? I will need to rank the results based on the most frequently appearing letters. Is using recursion in this scenerio generally a bad idea? Would iterating be a better choice?
s = "Hello there this is the string"
new_s = ""
for i in s:
if i != " ":
new_s += i
new_s = new_s.lower()
com_dict = {}
def merge_count(s):
global com_dict
if len(s) == 1:
if s[0] in com_dict:
com_dict[s[0]] += 1
else:
com_dict[s[0]] = 1
else:
merge_count(s[:int(len(s)/2)])
merge_count(s[int(len(s)/2):])
merge_count(new_s)
for i in com_dict:
print i, com_dict[i]Is the global variable necessary? Should I be using a different data structure instead of a dictionary? I will need to rank the results based on the most frequently appearing letters. Is using recursion in this scenerio generally a bad idea? Would iterating be a better choice?
Solution
Is the global variable necessary?
No. Global variables are almost never necessary, and rarely the best solution. Consider this:
In this, you pass a dictionary to merge into into to the function.
You do
Addition of strings in loops is always almost bad, because it may reallocate the string on every addition. Instead, one should do a join:
However, in this case you might as well just do
Recursively splitting in
which affords the better interface of
One can simplify things with
Or even further with
Instead of
One should do
Then one fixes up the naming
No. Global variables are almost never necessary, and rarely the best solution. Consider this:
def merge_count(s, com_dict):
if len(s) == 1:
if s[0] in com_dict:
com_dict[s[0]] += 1
else:
com_dict[s[0]] = 1
else:
merge_count(s[:int(len(s)/2)], com_dict)
merge_count(s[int(len(s)/2):], com_dict)
def main():
s = "Hello there this is the string"
new_s = ""
for i in s:
if i != " ":
new_s += i
new_s = new_s.lower()
com_dict = {}
merge_count(new_s, com_dict)
for i in com_dict:
print i, com_dict[i]
main()In this, you pass a dictionary to merge into into to the function.
You do
new_s = ""
for i in s:
if i != " ":
new_s += iAddition of strings in loops is always almost bad, because it may reallocate the string on every addition. Instead, one should do a join:
new_s = "".join(i for i in s if i != " ")However, in this case you might as well just do
new_s = s.replace(" ", "")Recursively splitting in
merge_count is pointless. Just dodef merge_count(s, com_dict):
for i in s:
if s in com_dict:
com_dict[s[0]] += 1
else:
com_dict[s[0]] = 1which affords the better interface of
def merge_count(s):
com_dict = {}
for i in s:
if s in com_dict:
com_dict[i] += 1
else:
com_dict[i] = 1
return com_dictOne can simplify things with
defaultdict:from collections import defaultdict
def merge_count(s):
com_dict = defaultdict(int)
for i in s:
com_dict[i] += 1
return com_dictOr even further with
Counter:from collections import Counter
def merge_count(s):
return Counter(s)Instead of
for i in com_dict:
print i, com_dict[i]One should do
for i, v in com_dict.iteritems():
print i, vThen one fixes up the naming
from collections import Counter
def main():
string = "Hello there this is the string"
lower_chars = string.replace(" ", "").lower()
char_counts = Counter(lower_chars)
for char, count in char_counts.items():
print(char, count)
main()Code Snippets
def merge_count(s, com_dict):
if len(s) == 1:
if s[0] in com_dict:
com_dict[s[0]] += 1
else:
com_dict[s[0]] = 1
else:
merge_count(s[:int(len(s)/2)], com_dict)
merge_count(s[int(len(s)/2):], com_dict)
def main():
s = "Hello there this is the string"
new_s = ""
for i in s:
if i != " ":
new_s += i
new_s = new_s.lower()
com_dict = {}
merge_count(new_s, com_dict)
for i in com_dict:
print i, com_dict[i]
main()new_s = ""
for i in s:
if i != " ":
new_s += inew_s = "".join(i for i in s if i != " ")new_s = s.replace(" ", "")def merge_count(s, com_dict):
for i in s:
if s in com_dict:
com_dict[s[0]] += 1
else:
com_dict[s[0]] = 1Context
StackExchange Code Review Q#94612, answer score: 9
Revisions (0)
No revisions yet.