2

I'm making a simple app that will spit out a random item from a list based on the category. I have two categories currently (places, and names). I'm trying to create two functions. One will return one of the categories randomly (working correctly) and one will take the random category and then randomly pull from the list for that category.

My problem is instead of the get_response function returning a value from the list, it returns a random character from the name of the list it is getting as an argument. Any ideas on how to get around this. Thanks

Here is my code:

from random import randrange

types = ["places", "names"]

places = ["The Upper Room", "Jerusalem", "Church", "Rome"]
names = ["Jesus", "Desciples", "Paul"]


def get_random_type():
    return str(types[randrange(0, len(types))])

def get_response(chosentype):
    return chosentype[randrange(0, len(chosentype))]

randtype = get_random_type()
print(randtype)
print(get_response(randtype))

EDIT: Thank you everyone for your help! What a great community!

Here is my final working code. It is a combination of multiple answers.

import random

categories = ["places", "names"]

options = dict(places = ["The Upper Room", "Jerusalem", "Church", "Rome"],
names = ["Jesus", "Desciples", "Paul"])


def get_random_type():
    return random.choice(categories)

def get_response(chosen_category):
    category_items = options[chosen_category]
    return random.choice(category_items)

print(get_response(get_random_type()))
luke bouch
  • 49
  • 5

4 Answers4

3

You can use a dictionary and random.choice to first select a random item from the types list, use that to key into the dictionary, then choice again to pick a random item from the list you picked:

>>> from random import choice
>>> types = ["places", "names"]
>>> options = dict(
...     places=["The Upper Room", "Jerusalem", "Church", "Rome"],
...     names=["Jesus", "Desciples", "Paul"]
... )
>>> choice(options[choice(types)])
Desciples

In your original code, chosentype[randrange(0, len(chosentype))] doesn't work because chosentype is a string, so you're essentially doing:

>>> "foobar"[randrange(0, len("foobar"))]
'o'

which doesn't make much sense. If you want to stick with this design, you're stuck keying into globals as if it were a dictionary, or writing an if-else branch, neither of which is as good as the above solution.

ggorlen
  • 44,755
  • 7
  • 76
  • 106
  • I don't see any reason why there is a need to explicitly change the input data structures just to solve OPs problem – Akshay Sehgal Aug 16 '20 at 02:43
  • There is. OP's code is poorly designed: the only way to solve the problem is to effectively abuse the `globals` structure as a dictionary instead of using your own, scoped and explicit dictionary. This involves resorting to `eval` or keying into globals (see the link) both of which are antipatterns. Branching using `if`-`else` is the least of those evils and would let OP keep their original code intact, but it's still ugly and inelegant. Instead of treating globals as a dict, write one explicitly. – ggorlen Aug 16 '20 at 02:45
  • So your assumption being that OP may not be giving a toy example? maybe there are 200 types and each of the 200 have a list? OP should sit and make a dict like you have recommended just to solve this problem this way? – Akshay Sehgal Aug 16 '20 at 02:47
  • That's precisely my suggestion of how to solve the problem. Of course, if they're stuck with a bunch of loose globals (unlikely), they should collect them programmatically into a dict as soon as possible. When you have variables that need to be manipulated as a unit, they should be in the correct [data structure](https://en.wikipedia.org/wiki/Data_structure) that supports the operation such as a dictionary. Loose variables in `globals` cannot be cleanly selected from by key as your answer shows. Dictionaries are a key-value structure that solves OP's problem cleanly. – ggorlen Aug 16 '20 at 02:52
  • Do consider that OP (or someone with a similar problem) may not be able to just collect variables into a dict, specially if they are coming from different data sources. Given that the loose variables are already designed that way, what's available set of solutions are what we target to suggest. I am in no way saying your solution is wrong, i dont see what makes mine incorrect or a hack. – Akshay Sehgal Aug 16 '20 at 02:55
  • 1
    That makes so much sense! The code I included above is my entire project so it will be easy to implement. I’m new to python (only a couple of weeks) and came up with the idea for this app. It will ultimately pull from a MySQL database and packaged into an iOS app (I still have a long way to go). I want to use a database so I can actively add items to be random spit out. Thank you so much for all your help! Such a great community – luke bouch Aug 17 '20 at 02:03
  • @lukebouch No problem, happy to help. You can see even with a small chunk of code there are some significant design decisions and debates to be had, so it's an interesting question. Feel free to check [someone answers](https://stackoverflow.com/help/someone-answers) if you haven't yet. – ggorlen Aug 17 '20 at 02:09
2

Use a dictionary. Here's a fully functional working code that meets your requirement with least changes to make in your code.

from random import randrange

config = {
   "places": ["The Upper Room", "Jerusalem", "Church", "Rome"],
   "names": ["Jesus", "Desciples", "Paul"]
}

def get_random_type():
    types = list(config.keys())
    return str(types[randrange(0, len(types))])

def get_response(chosentype):
    return config[chosentype]

randtype = get_random_type()
print(randtype)
print(get_response(randtype))
Saurav Pathak
  • 796
  • 11
  • 32
1

The issue is that in the first function you are passing a iterator (list) while in the second you are passing a string as iterator, so the string "places" is being used to get a random value from, which returns a character.

I would recommend using random.choice and eval function to solve this in a more elegant way -

import random

def get_random_type():
    return random.choice(types)

def get_response(chosentype):
    return random.choice(eval(chosentype))

randtype = get_random_type()
print(randtype)
print(get_response(randtype))
places
The Upper Room

So, if the first function chooses 'places', then eval function over 'places' will return variable places which is the list of places. Using another random.choice over that will give you a random place.

EDIT: As pointed out, eval function may be dangerous as it can allow someone to write in a string of code as input forcing your app to run the code. HOWEVER, it's dangerous only if it's used irresponsibly. In this case, since the first function is forced to return a choice from a predefined list, the input to the second function is in safe hands. if you still want to ensure safety, you can do the following

def get_response(chosentype):
    if chosentype in types:
        return random.choice(eval(chosentype))

Please read about eval, its misuse, and advantages if you plan to set this up as part of a webapp.

Akshay Sehgal
  • 18,741
  • 3
  • 21
  • 51
  • `random.choice` is far better than resorting to `eval` simply to pick something from a list. Having to use `eval` reveals the fundamental design flaw, which is that a string variable name is being passed into `get_response` when the list itself should be passed in. Programs should never "know" what their variable names are. – ggorlen Aug 16 '20 at 02:37
  • ```eval``` is not used in place of ```random.choice``` it's used to reference a variable which is returned as a string from the previous function. And you are absolutely wrong about the "fundamental design flaw". this is the rare use case where ```eval``` function actually shines. – Akshay Sehgal Aug 16 '20 at 02:40
  • do you know what ```eval``` does here? it gets a string like "places" and returns the list of places for random.choice to operate on. how is it being abused? – Akshay Sehgal Aug 16 '20 at 02:42
  • See my above comments. The reason `eval` works is because a variable name in the global scope happens to match the string passed into the function. That's brittle at best and a security issue at worst. Worst of all, it's simply not necessary if you use a dictionary to store the target lists by key. – ggorlen Aug 16 '20 at 02:44
  • Again, you are modifying the problem statement that OP has mentioned in the question. You can simply say that please store your data in a dict instead of a list?! what if the data comes from another source? OP mentioned that he has 1 list that contains the "types" and each of the "types" have their own lists of options. THAT is the premise of the question, you are not allowed to change the premise just to fit a better solution. – Akshay Sehgal Aug 16 '20 at 02:46
  • Why not? OP hasn't specified that their design is set in stone. Better to fix the fundamental problem in the design up front than start piling on brittle hacks. If their design _is_ set in stone, then `if`-`else` is still better than `eval`ing on `globals`. – ggorlen Aug 16 '20 at 02:49
  • You can recommend changes to the premise of the problem in YOUR solution sure. I am not suggesting otherwise. But the exact solution that considers the given premise of inputs and the expected output is the better solution. You can just call using ```eval``` here a brittle hack or a design flaw. That is spreading misinformation and being extremely rude at the same time. – Akshay Sehgal Aug 16 '20 at 02:51
  • Feel free to do [some research on `eval` if you don't believe me](https://stackoverflow.com/a/1832957/6243352). It's unsafe and almost never necessary, and having to use it indicates a serious fundamental design flaw 99 times out of 99. Nothing personal, but what you're recommending OP is dangerous design so I had to mention it. Thanks for the conversation but I need to move on to other things. Using your code, you can wipe a harddrive with unsanitized user input. – ggorlen Aug 16 '20 at 02:55
  • The link that you mention for research on eval, the first top comment on that is ""Very dangerous and insecure" is false..."". That is my argument here as well. Please read the EDIT 2 that is mentioned in the linked answer itself. – Akshay Sehgal Aug 16 '20 at 03:06
  • The comment is nuanced: they're saying `eval` _itself_ isn't unsafe in the same way people say "guns don't kill people, people kill people". But the truth is that guns make it much easier to kill someone in the same way `eval` is a design crutch and makes it much easier to let your app be exploited. Read the follow-up comments. I'd never recommend a beginner use it to fix their design problem. You've now had to add a conditional just to "sanitize" the `eval`. If OP persists with this design, they'll need to `eval` and sanitize every time they want to access the structures. – ggorlen Aug 16 '20 at 03:12
  • 2
    Even if `eval` was 100% safe, as `ast.literal_eval` is (as far as I know), it's still making up for a shoddy design that puts a bunch of related variables that should be collected in a dictionary loose in the global scope and passes a string var name into a function that expects a list, breaking encapsulation. If you say OP is stuck with this design, that's imaginary until confirmed. From the looks of it, it's a first-time programmer not dealing with legacy code, and even if they are dealing with legacy code, best to gather those vars into a dict ASAP so they can program safely and cleanly. – ggorlen Aug 16 '20 at 03:14
  • Anyway, I really need to move on. Nice chatting. – ggorlen Aug 16 '20 at 03:14
  • That is a very abstract statement to make for this case. If OP is a new programmer, I don't see why he should be too worried about 'safety' of a toy example that he is attempting to make. Secondly, in this specific case, ```eval``` doesn't pose a threat, to begin with... Also, likewise, nice chatting on this topic. – Akshay Sehgal Aug 16 '20 at 03:16
1

randtype is a string not the list. You could do

print(get_response(eval(randtype)))

eval function evaluates the “String” like a "python expression" and therefore eval(randtype) is the list and not the string.

aerijman
  • 2,522
  • 1
  • 22
  • 32