10

I've generally heard that using global variables is a bad practice when programming. So are there any alternatives to what I'm trying to do here, which is to use the variable my_array from inside another function?

import random

def main():  
    create_list_and_find_max_and_min(10)  
    the_smart_way()

def create_list_and_find_max_and_min(n):
    global my_array
    my_array = []
    n = input("How many numbers do you want in your array?:")
    for i in range(n):
        my_array.append(random.randint(1,n))
    print "My array is:", my_array

    min = my_array[0]
    for number in my_array:
    if min > number:
        min = number
    print "The minimum value in the array is:", min

    max = my_array[0]
    for number in my_array:
        if max < number:
        max = number
    print "The maximum value in the array is:", max

def the_smart_way():
    # "This one uses the built-in Python functions for min/max..."
    min_my_array = min(my_array)
    max_my_array = max(my_array)
    return min_my_array, max_my_array

if __name__ == '__main__':
    main()
mkrieger1
  • 19,194
  • 5
  • 54
  • 65
Shankar Kumar
  • 2,197
  • 6
  • 25
  • 32

3 Answers3

18

Yes, there are two alternatives.

First, you can pass the values around instead of using globals. For example, create_list_and_find_max_and_min can create the array locally and return it, then you can pass it in to the_smart_way:

import random  

def main():  
    my_array = create_list_and_find_max_and_min(10)  
    print the_smart_way(my_array)

def create_list_and_find_max_and_min(n):
    my_array = []
    n = input("How many numbers do you want in your array?:")
    for i in range(n):
        my_array.append(random.randint(1,n))
    print "My array is:", my_array

    min = my_array[0]
    for number in my_array:
        if min > number:
            min = number
    print "The minimum value in the array is:", min

    max = my_array[0]
    for number in my_array:
        if max < number:
            max = number
    print "The maximum value in the array is:", max
    return my_array

def the_smart_way(my_array):
    # "This one uses the built-in Python functions for min/max..."
    min_my_array = min(my_array)
    max_my_array = max(my_array)
    return min_my_array, max_my_array

if __name__ == '__main__':
    main()

Second, you can create a class that encapsulates the data and the functions that operate on that data:

import random  

class MyArrayClass(object):
    def create_list_and_find_max_and_min(self, n):
        self.my_array = []
        n = input("How many numbers do you want in your array?:")
        for i in range(n):
            self.my_array.append(random.randint(1,n))
        print "My array is:", self.my_array

        min = self.my_array[0]
        for number in self.my_array:
            if min > number:
                min = number
        print "The minimum value in the array is:", min

        max = self.my_array[0]
        for number in self.my_array:
            if max < number:
                max = number
        print "The maximum value in the array is:", max

    def the_smart_way(self):
        # "This one uses the built-in Python functions for min/max..."
        min_my_array = min(self.my_array)
        max_my_array = max(self.my_array)
        return min_my_array, max_my_array

def main():
    my_array = MyArrayClass()
    my_array.create_list_and_find_max_and_min(10)  
    print my_array.the_smart_way()

if __name__ == '__main__':
    main()

You should probably understand the reasons global variables are bad practice.

Imagine that you want to create two arrays. With global variables, the second one will replace the first one, which will be gone forever.

create_list_and_fix_max_and_min(10)
create_list_and_fix_max_and_min(20)
# No way to operate on the original array!

With a local variable, you can store both of them:

my_array_1 = create_list_and_fix_max_and_min(10)
my_array_2 = create_list_and_fix_max_and_min(20)
the_smart_way(my_array_1)

Using an object provides the same benefit; the difference between the two ultimately comes down to whether the operations are part of the meaning of the data, or whether the data stand alone and the operations are generic. (Or, sometimes, whether you're more of a functional snob or an OO snob…)

mkrieger1
  • 19,194
  • 5
  • 54
  • 65
abarnert
  • 354,177
  • 51
  • 601
  • 671
  • Hi, I'm getting an error when I try it using your first method, it says the global variable my_array is not defined "line 3." – Shankar Kumar Jul 13 '12 at 00:12
  • I didn't post the complete code, hence the #... bits. I'll edit it to include the whole thing; give me a minute. – abarnert Jul 13 '12 at 00:14
2

Functions do things to objects and then return results. You want to keep functions dead-simple and perform all of your logic and processing outside of the function. This will remove the need for global variables and will make your code much, much easier to read.

That being said, here is how I would attack your problem:

import random  

def random_list(n=None):
  n = n or int(raw_input('How many numbers do you want in your list? '))    

  return [random.randint(1, n) for i in range(n)]

if __name__ == '__main__':
  my_list = random_list(10)
  minimum, maximum = min(my_list), max(my_list)

  print 'My list is ', my_list
  print 'The minimum value in the list is ', minimum
  print 'The maximum value in the list is ', maximum
Blender
  • 289,723
  • 53
  • 439
  • 496
1

Here is how I would do this:

import random  

def main():
    # note that input can be dangerous since it evaluates arbitrary code
    n = int(raw_input("How many numbers do you want in your array?: "))
    my_list = [random.randint(1, n) for _ in range(n)]
    find_max_and_min(my_list)  
    the_smart_way(my_list)

def find_max_and_min(seq):
    print "My array is:", seq

    min_num = seq[0] # Don't want to use same names as bultins here
    for number in seq:
        if number < min_num:
            min_num = number
    print "The minimum value in the array is:", min_num

    max_num = seq[0]
    for number in seq:
        if number > max_num:
            max_num = number
    print "The maximum value in the array is:", max_num

def the_smart_way(seq):
    # "This one uses the built-in Python functions for min/max..."
    # No need for temp variables here
    print min(seq), max(seq)

if __name__ == '__main__':
    main()
mkrieger1
  • 19,194
  • 5
  • 54
  • 65
jamylak
  • 128,818
  • 30
  • 231
  • 230
  • I concentrated on how to change his code as little as possible to avoid confusion, but your refactoring makes his code much nicer in general. (But note that his the_smart_way returns min, max rather than printing them.) – abarnert Jul 13 '12 at 00:03
  • @abarnert Thanks, I assumed that it was supposed to print them but it doesn't really matter. – jamylak Jul 13 '12 at 00:04