1
\$\begingroup\$

Any efficient way to solve the following problem assuming data is large. I solved the problem but how can I improve the code, which will make it efficient. any suggestions?

Data:

movie_sub_themes = {
'Epic': ['Ben Hur', 'Gone With the Wind', 'Lawrence of Arabia'],
'Spy': ['James Bond', 'Salt', 'Mission: Impossible'],
'Superhero': ['The Dark Knight Trilogy', 'Hancock, Superman'],
'Gangster': ['Gangs of New York', 'City of God', 'Reservoir Dogs'],
'Fairy Tale': ['Maleficent', 'Into the Woods', 'Jack the Giant Killer'],
'Romantic':['Casablanca', 'The English Patient', 'A Walk to Remember'],
'Epic Fantasy': ['Lord of the Rings', 'Chronicles of Narnia', 'Beowulf']}

movie_themes = {
'Action': ['Epic', 'Spy', 'Superhero'],
'Crime' : ['Gangster'],
'Fantasy' : ['Fairy Tale', 'Epic Fantasy'],
'Romance' : ['Romantic']}

themes_keys = movie_themes.keys()
theme_movies_keys = movie_sub_themes.keys()

#Iterate in movie_themes
#Check movie_themes keys in movie_sub_keys
#if yes append the movie_sub_keys into the newdict
newdict = {}
for i in range(len(themes_keys)):
   a = []
   for j in range(len(movie_themes[themes_keys[i]])):
     try:
         if movie_themes[themes_keys[i]][j] in theme_movies_keys:
            a.append(movie_sub_themes[movie_themes[themes_keys[i]][j]])
     except:
         pass
   newdict[themes_keys[i]] = a

# newdict contains nested lists
# Program to unpack the nested list into single list
# Storing the value into theme_movies_data 
theme_movies_data = {}
for k, v in newdict.iteritems():
    mylist_n = [j for i in v for j in i]
    theme_movies_data[k] = dict.fromkeys(mylist_n).keys()

print (theme_movies_data)

Output:

{'Action': ['Gone With the Wind', 'Ben Hur','Hancock, Superman','Mission: Impossible','James Bond','Lawrence of Arabia','Salt','The Dark Knight Trilogy'],
 'Crime': ['City of God', 'Reservoir Dogs', 'Gangs of New York'],
 'Fantasy': ['Jack the Giant Killer','Beowulf','Into the Woods','Maleficent','Lord of the Rings','Chronicles of Narnia'],
 'Romance': ['The English Patient', 'A Walk to Remember', 'Casablanca']}

Apologies for not properly commenting the code.

I am more concern about the running time.

\$\endgroup\$
  • 2
    \$\begingroup\$ Welcome to Code Review. Your title should explain what your code does on this site. codereview.stackexchange.com/help/how-to-ask \$\endgroup\$ – chicks May 22 '18 at 15:27
  • 1
    \$\begingroup\$ "Any efficient way to solve the following problem"… Which problem? \$\endgroup\$ – Mathias Ettinger May 22 '18 at 15:38
3
\$\begingroup\$
  1. theme_movies_data and newdict are bad variable names, change them to ones easier to read. This will reduce the amount of comments you need in your code.
  2. You can simplify your code if you stop using range and use dict.iteritems more.
  3. You shouldn't need your try. You would know this if you use range less.
  4. You don't need dict.fromkeys(mylist_n).keys() it's just useless.
new_dict = {}
for key, themes in movie_themes.items():
    a = []
    for theme in themes:
        if theme in movie_sub_themes:
            a.append(movie_sub_themes[theme])
    new_dict[key] = a


theme_movies = {}
for key, movie_groups in new_dict.iteritems():
    theme_movies_data[key] = [
        movie
        for movies in movie_groups
        for movie in movies
    ]

print(theme_movies)
  1. You can remove the need for the second loop if you use a.extend.
  2. You can change the creation of a to a comprehension.
  3. You can change the creation of theme_movies to a dictionary comprehension.
theme_movies = {
    key: sum(
        movie_sub_themes.get(theme, [])
        for theme in themes
    )
    for key, themes in movie_themes.iteritems()
}

print(theme_movies)

Alternately if you don't like sum:

theme_movies = {
    key: [
        movie
        for theme in themes
        for movie in movie_sub_themes.get(theme, [])
    ]
    for key, themes in movie_themes.iteritems()
}

print(theme_movies)
\$\endgroup\$
1
\$\begingroup\$

Here's my solution (using defaultdict):

movie_sub_themes = {
'Epic': ['Ben Hur', 'Gone With the Wind', 'Lawrence of Arabia'],
'Spy': ['James Bond', 'Salt', 'Mission: Impossible'],
'Superhero': ['The Dark Knight Trilogy', 'Hancock, Superman'],
'Gangster': ['Gangs of New York', 'City of God', 'Reservoir Dogs'],
'Fairy Tale': ['Maleficent', 'Into the Woods', 'Jack the Giant Killer'],
'Romantic':['Casablanca', 'The English Patient', 'A Walk to Remember'],
'Epic Fantasy': ['Lord of the Rings', 'Chronicles of Narnia', 'Beowulf']}

movie_themes = {
'Action': ['Epic', 'Spy', 'Superhero'],
'Crime' : ['Gangster'],
'Fantasy' : ['Fairy Tale', 'Epic Fantasy'],
'Romance' : ['Romantic']}

from collections import defaultdict
newdict = defaultdict(list)

for theme, sub_themes_list in movie_themes.items():
    for sub_theme in sub_themes_list:
        newdict[theme] += movie_sub_themes.get(sub_theme, [])       

dict(newdict)

>> {'Action': ['Ben Hur',
  'Gone With the Wind',
  'Lawrence of Arabia',
  'James Bond',
  'Salt',
  'Mission: Impossible',
  'The Dark Knight Trilogy',
  'Hancock, Superman'],
 'Crime': ['Gangs of New York', 'City of God', 'Reservoir Dogs'],
 'Fantasy': ['Maleficent',
  'Into the Woods',
  'Jack the Giant Killer',
  'Lord of the Rings',
  'Chronicles of Narnia',
  'Beowulf'],
 'Romance': ['Casablanca', 'The English Patient', 'A Walk to Remember']}

timings: 4.84 µs vs 14.6 µs

\$\endgroup\$
  • 1
    \$\begingroup\$ Since it's Python 2.7, consider using iteritems instead of items. Also the immutable data at the top should be using tuples instead of lists. \$\endgroup\$ – Reinderien May 22 '18 at 18:41
  • \$\begingroup\$ Also, in your inner loop, avoid writing newdict[theme]. You should cache the result of this index lookup in the level above. \$\endgroup\$ – Reinderien May 22 '18 at 18:43

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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