Should I use a global variable (array in this case) or should i return the array

[rupeshpradhan]

I am using an array 'array_studentDetails'. This array will be used by other functions after it is initialized.

Should I make it a global array as I have done in this example
OR
should i return the array to the calling function and pass it on to the other function that require them as a parameter?

# initialize global variables
array_studentDetails = []

def populate_array(allRecords):
    # validate parameters
    # TODO

    # initilize local variables
    numberOfRecords = 0

    # initialize array
    for record in allRecords:
        numberOfRecords += 1

        #Format : e = ['1','name']
        oneArrayElement = [record[0], record[1]]
        array_studentDetails.append(oneArrayElement)

    return numberOfRecords

function test
-------------
allRecords = [(1,'rupesh'), (2, 'upesh')]

populate_array(allRecords)
2

array_studentDetails[0][0]
1

array_studentDetails[0][1]
'rupesh'

[MK12]

First of all, python doesn't use arrays, what you're referring to is called list. You should go with the option that produces the cleanest code. I'm not really sure because you haven't given much information, I'd need to see the whole module to give you an opinion. But if the list is going to be used by many methods, then I'd opt for the global variable. If only two, maybe the arguments.

[rupeshpradhan]

This is probably the tenth time or more, i am re-writing this module. Each time I am trying to make it as modular as possible (if that is the word i am supposed to use, indicating what i am trying to do!). What I am trying to do is remove as much as possible the "hard coding" that i have done (to get the program to work in a quick and dirty manner) into function, parameter and returns.

I will probably finish it by tomorrow and will put up the completed module at the same time as today.

Throw at me as much comments as you can.

I am planning to use Python for a bigger project, and i need to polish my coding style and standardize it as much as possible.

In fact, you can help me out in standardising my coding style (i.e. variable names, function names, classes names, methods names etc. etc.)

[sneekula]

Since a list is a mutable object, it would still behave global if it is used as a function parameter/argument. To wit:

def add_element(mylist2):
    mylist2.append(33)
    return mylist2

mylist = [1,2,3,4]
mylist3 = add_element(mylist)

print(mylist)   # [1, 2, 3, 4, 33]  oops!
print(mylist3)  # [1, 2, 3, 4, 33]

To prevent mutable parameter feedback use:

def add_element(mylist2):
    # make a copy to prevent feedback
    mylist2 = list(mylist)
    mylist2.append(33)
    return mylist2

mylist = [1,2,3,4]
mylist3 = add_element(mylist)

print(mylist)   # [1, 2, 3, 4]
print(mylist3)  # [1, 2, 3, 4, 33]

[MK12]

This is probably the tenth time or more, i am re-writing this module. Each time I am trying to make it as modular as possible (if that is the word i am supposed to use, indicating what i am trying to do!). What I am trying to do is remove as much as possible the "hard coding" that i have done (to get the program to work in a quick and dirty manner) into function, parameter and returns.

I will probably finish it by tomorrow and will put up the completed module at the same time as today.

Throw at me as much comments as you can.

I am planning to use Python for a bigger project, and i need to polish my coding style and standardize it as much as possible.

In fact, you can help me out in standardising my coding style (i.e. variable names, function names, classes names, methods names etc. etc.)

Definitely check out this python PEP: Python Enhancement Proposal 8.

[rupeshpradhan]

After 8 hours of work, here it is!
comments please! (specially on how i could improve upon my coding style and the way i am coding)

'''
USAGE : prepare_input_sheet(className, allRecords, headerType, allowCondensedMode, namesPerColumn, fillInRollNumber, session)

className : type string, name of class e.g. '9A'
allRecords : type list, contains tuple of roll number and name e.g. [(1, 'name of student')]
headerType : type int, selects the header type for input sheet, range starting from 1
allowCondensedMode : type bool, determined whether to use the condensed mode for dot matrix printer, if false columns are not more than 2
namesPerColumn : type int, number of rows in one column, range starting from 1 generally upto 30
fillInRollNumber : type bool; determined whether the blank roll number spaces should be filled automatically; generally used when all students are present in the sheet
session : type string; four digit year YYYY
'''

# imports
import pyodbc

# initialize global variables
array_studentDetails = []

# module settings
nameWidth = 16
rollNumberWidth = 3
pageWidth = 80

# toggle settings
printerControlCode = 1

# toggle settings implementation
# - printer control codes
if printerControlCode:
    condensedMode_ON = chr(15)
    condensedMode_OFF = chr(18)
else:
     condensedMode_ON = ''
     condensedMode_OFF = ''

# constants
lineBreak = '\r\n'
pageBreak = chr(12)



# function re-usable comments
# validate parameters
# TODO

# initialize local varaiables


def populate_array(allRecords):
    # validate parameters
    # TODO
    # validations
    # - must not be empty
    # - be type list

    # initilize local variables
    numberOfRecords = 0

    # initialize array
    for record in allRecords:
        numberOfRecords += 1

        #Format : e = ['1','name']
        oneArrayElement = [record[0], record[1]]
        array_studentDetails.append(oneArrayElement)

    return numberOfRecords


def get_number_of_pages(numberOfRecords, allowCondensedMode):
    # validate parameters
    # TODO
    # validations
    # - type positive integer
    # - maximum range limit for now is 180 as values are hard coded

    # initialize local varaiables
    numberOfPages = 0
    condensedMode = 0

    # calculate numberOfColumn
    if allowCondensedMode:

        if numberOfRecords > 0 and numberOfRecords < 61:
            numberOfPages = 1
            condensedMode = 0

        if numberOfRecords > 60 and numberOfRecords < 121:
            numberOfPages = 1
            condensedMode = 1
    else:
        if numberOfRecords > 0 and numberOfRecords < 61:
            numberOfPages = 1

        if numberOfRecords > 60 and numberOfRecords < 121:
            numberOfPages = 2

        if numberOfRecords > 120 and numberOfRecords < 181:
            numberOfPages = 3

    return numberOfPages, condensedMode



def get_number_of_columns(numberOfRecords, allowCondensedMode=0):
    # validate parameters
    # TODO

    # initialize local varaiables
    numberOfColumns = 0

    if allowCondensedMode:
        # set numberOfColumn based on numberOfRecords
        if numberOfRecords > 0 and numberOfRecords < 61:
            numberOfColumns = 2
        if numberOfRecords > 60 and numberOfRecords < 91:
            numberOfColumns = 3
        if numberOfRecords > 90 and numberOfRecords < 121:
            numberOfColumns = 4
    else:
        numberOfColumns = 2 # default is 2 anyway
        #numberOfColumns = 1

    return numberOfColumns


def header_content(headerType, session):
    # -- define column header : CAT for Senior School
    if headerType == 1:
        title_1 = "ROCKVALE ACADEMY, KALIMPONG.             Continuous Assessment Test CAT ____" + session
        title_2 = "Teacher's Name:.........................Subject:................................"
        rowSeparator = "|" + "-" * rollNumberWidth + "|" + "-" * nameWidth +               "|- - -|- - -|- - -|"
        columnHeader = "|"+ "Rn".ljust(rollNumberWidth) + "|" + "Name".center(nameWidth) + "|CAT 1|CAT 2|Total|"
        blankMarksColumn =                                                                 "|     |     |     |"

    if headerType == 2:
        title_1 = "ROCKVALE ACADEMY, KALIMPONG.                        Half Yearly Examination " + session
        title_2 = "Teacher's Name:.........................Subject:................................"
        rowSeparator = "|" + "-" * rollNumberWidth + "|" + "-" * nameWidth +               "|- - -|- - -|- - -|"
        columnHeader = "|"+ "Rn".ljust(rollNumberWidth) + "|" + "Name".center(nameWidth) + "|      MARKS      |"
        blankMarksColumn =                                                                 "|                 |"

    if headerType == 3:
        title_1 = "ROCKVALE ACADEMY, KALIMPONG.             Continuous Assessment Test CAT ____" + session
        title_2 = "Teacher's Name:.........................Subject:................................"
        rowSeparator = "|" + "-" * rollNumberWidth + "|" + "-" * nameWidth +               "|- - -|- - -|- - -|"
        columnHeader = "|"+ "Rn".ljust(rollNumberWidth) + "|" + "Name".center(nameWidth) + "|CAT 3|CAT 4|Total|"
        blankMarksColumn =                                                                 "|     |     |     |"

    if headerType == 4:
        title_1 = "ROCKVALE ACADEMY, KALIMPONG.                             Annual Examination " + session
        title_2 = "Teacher's Name:.........................Subject:................................"
        rowSeparator = "|" + "-" * rollNumberWidth + "|" + "-" * nameWidth +               "|- - -|- - -|- - -|"
        columnHeader = "|"+ "Rn".ljust(rollNumberWidth) + "|" + "Name".center(nameWidth) + "|      MARKS      |"
        blankMarksColumn =                                                                 "|                 |"

    if headerType == 5:
        title_1 = "ROCKVALE ACADEMY, KALIMPONG.             Acknowledgement Sheet              " + session
        title_2 = "Particulars:..................................................................."
        rowSeparator = "|" + "-" * rollNumberWidth + "|" + "-" * nameWidth +               "|- - -|- - -|- - -|"
        columnHeader = "|"+ "Rn".ljust(rollNumberWidth) + "|" + "Name".center(nameWidth) + "|   FIRST NAME    |"
        blankMarksColumn =                                                                 "|                 |"

    if headerType == 6:
        title_1 = "ROCKVALE ACADEMY, KALIMPONG.             Unit Test I                        " + session
        title_2 = "Teacher's Name:.........................Subject:................................"
        rowSeparator = "|" + "-" * rollNumberWidth + "|" + "-" * nameWidth +               "|- - -|- - -|- - -|"
        columnHeader = "|"+ "Rn".ljust(rollNumberWidth) + "|" + "Name".center(nameWidth) + "|     MARKS       |"
        blankMarksColumn =                                                                 "|                 |"

    if headerType == 7:
        title_1 = "ROCKVALE ACADEMY, KALIMPONG.             Unit Test II                       " + session
        title_2 = "Teacher's Name:.........................Subject:................................"
        rowSeparator = "|" + "-" * rollNumberWidth + "|" + "-" * nameWidth +               "|- - -|- - -|- - -|"
        columnHeader = "|"+ "Rn".ljust(rollNumberWidth) + "|" + "Name".center(nameWidth) + "|     MARKS       |"
        blankMarksColumn =                                                                 "|                 |"
    return title_1, title_2, rowSeparator, columnHeader, blankMarksColumn


def get_row_separator(headerType, session):
    rowSeparator = header_content(headerType, session)[2]

    return rowSeparator


def get_column_header(headerType, session):
    columnHeader = header_content(headerType, session)[3]

    return columnHeader


def get_title(headerType, session, titleNumber):
    if titleNumber == 0:
        title = header_content(headerType, session)[0]
    elif titleNumber == 1:
        title = header_content(headerType, session)[1]

    return title

def get_blank_marks_column(headerType, session):
    blankMarksColumn = header_content(headerType, session)[4]

    return blankMarksColumn


def get_one_array_element(row):
    try:
        roll = array_studentDetails[row][0]
        name = array_studentDetails[row][1]
    except:
        roll = " " * rollNumberWidth
        name = " " * nameWidth

    return roll, name


def row_content(row, numberOfColumns, headerType, namesPerColumn, pageNumber, fillInRollNumber, session):
    # validate parameters
    # TODO
    # - row is positive integer less than number of rows per page
    # - numberOfColumns is postive integer less than max number of columns

    # initialize local varaiables
    rowContent = ''
    columnContent = ''
    elementIndex = 0


#    for row in range (0, namesPerColumn):
    for column in range(0, numberOfColumns):
        elementIndex = (column * namesPerColumn) + (pageNumber * 2 * namesPerColumn) + (row)
        roll, name = get_one_array_element(elementIndex)

        # fill in empty roll number columns
        if fillInRollNumber:
            roll = elementIndex + 1

        name = name[0:nameWidth].ljust(nameWidth,' ')
        columnContent = "|" + (str(roll)).rjust(rollNumberWidth) + "|" + name + get_blank_marks_column(headerType, session)
        rowContent += columnContent

##    if numberOfColumns == 1:
##        roll1, name1 = get_one_array_element(row + (namesPerColumn * pageNumber))
##        name1 = name1[0:nameWidth].ljust(nameWidth,' ')
##        rowContent1 = "|" + (str(row + (namesPerColumn * pageNumber) + 1)).rjust(rollNumberWidth) + "|" + name1 + get_blank_marks_column(headerType)
##
##    if numberOfColumns == 2:
##        roll1, name1 = get_one_array_element(row + (namesPerColumn* 2 * pageNumber))
##        name1 = name1[0:nameWidth].ljust(nameWidth,' ')
##        rowContent1 = "|" + (str(row + (namesPerColumn * 2 * pageNumber) + 1)).rjust(rollNumberWidth) + "|" + name1 + get_blank_marks_column(headerType)
##
##        roll2, name2 = get_one_array_element(row + (namesPerColumn * 2 * pageNumber) + namesPerColumn)
##        name2 = name2[0:nameWidth].ljust(nameWidth,' ')
##        rowContent2 = "|" + (str(row + (namesPerColumn * 2 * pageNumber) + 1 + namesPerColumn)).rjust(rollNumberWidth) + "|" + name2 + get_blank_marks_column(headerType)
##
##    rowContent = rowContent1 + rowContent2


    return rowContent


def make_one_page(pageNumber, numberOfRecords, condensedMode, headerType, className, namesPerColumn, fillInRollNumber, session):
    # validate parameters
    # TODO

    # initialize local variables
    onePageContent = ""

    # set condensed mode printer control code ON if needed
    if condensedMode:
        onePageContent = onePageContent + condensedMode_ON

    # get number of columns FNDONE
    numberOfColumns = get_number_of_columns(numberOfRecords, condensedMode)

    # set row separator FNDONE
    rowSeparator = get_row_separator(headerType, session) * numberOfColumns

    # set column header FNDONE
    columnSeparator = get_column_header(headerType, session) * numberOfColumns

    # prepare header
    onePageContent += get_title(headerType, session, 0) + lineBreak  # FNDONE
    onePageContent += 'Class : ' + className + lineBreak
    onePageContent += get_title(headerType, session, 1) + lineBreak  # FNDONE
    onePageContent += rowSeparator + lineBreak
    onePageContent += columnSeparator + lineBreak
    onePageContent += rowSeparator + lineBreak

    # add rows
    for row in range(0, namesPerColumn):
        onePageContent += row_content(row, numberOfColumns, headerType, namesPerColumn, pageNumber, fillInRollNumber, session) + lineBreak # FNDONE - row_content()
        onePageContent += rowSeparator + lineBreak

    # set condensed mode printer control code OFF if needed
    if condensedMode:
        onePageContent = onePageContent + condensedMode_OFF

    # add page break
    onePageContent += pageBreak

    return onePageContent


def prepare_input_sheet(className, allRecords, headerType, allowCondensedMode, namesPerColumn, fillInRollNumber, session):
    # validate parameters
    # TODO

    # initialize local variables
    content = ""

    # get number of records FNDONE
    numberOfRecords = populate_array(allRecords)

    # get number of pages FNDONE
    numberOfPages, condensedMode = get_number_of_pages(numberOfRecords, allowCondensedMode)

    # iterate through number of pages
    for pageNumber in range(0, numberOfPages):
        content = content + make_one_page(pageNumber, numberOfRecords, condensedMode, headerType, className, namesPerColumn, fillInRollNumber, session) # FNDONE

    return content


def write_to_file(fileName, content):
    f = open(fileName, 'w')
    f.write(content)
    f.close()

# main

if __name__ == '__main__':

    allRecords = [(1, 'ABHISHEAK SHAH'),
     (2, 'AKASH BASNET'),
     (3, 'ALACE RAI'),
     (4, 'ALASH RAI'),
     (5, 'ANISHA BASHEER'),
     (6, 'ASHISH SARDA'),
     (7, 'ASHISH SUBEDI'),
     (8, 'ASHUTOSH RANJAN'),
     (9, 'ASHWIN PRADHAN'),
     (10, 'AZHARUL ISLAM'),
     (11, 'DANISH TANVEER'),
     (12, 'DENZIL BHUTIA'),
     (13, 'JAMBA DOMA BISTA'),
     (14, 'JENNIFER SINGH'),
     (15, 'JORDEN LAMA'),
     (16, 'K.H.M. RIAZUDDIN RABBI'),
     (17, 'KARMA THUTOPE NAMGYEL'),
     (18, 'KENNETH NATHAN BOR ADHIKARI'),
     (19, 'KUNZANG TOPGAY LACHUNGPA'),
     (20, 'LAWANG DORJEE WAIBA'),
     (21, 'M.H. BAQUER ALI SEWITZ'),
     (22, 'MAMUNUR RASHID'),
     (23, 'MANISH GURUNG'),
     (24, 'MANISHA RAI'),
     (25, 'MINGMA LAMA'),
     (26, 'MOIRANGTHEM RONNIE SINGH'),
     (27, 'MUSKAN SETHIA'),
     (28, 'NIKITA REMTLUANGPUII'),
     (29, 'OMISA PRADHAN'),
     (30, 'PALDEN WANGCHUK'),
     (31, 'PINKY KIPGEN'),
     (32, 'PRABIJAN GURUNG'),
     (33, 'PRASAN PRADHAN'),
     (34, 'PRAVIN GAUTAM'),
     (35, 'PREETISA SUBBA'),
     (36, 'PRIYANKA THAPA (GURUNG)'),
     (37, 'PURNIMA PAKHRIN'),
     (38, 'RAKESH CHAUDHARY'),
     (39, 'RINCHEN DOLMA'),
     (40, 'RINZING SANGMO'),
     (41, 'RISHAV MITRA'),
     (42, 'SABIN PRADHAN'),
     (43, 'SAROJ DUTT KISKU'),
     (44, 'SAUGAT MUKHIA'),
     (45, 'SHIVANI PRADHAN'),
     (46, 'SHUBHAM GUPTA'),
     (47, 'SIDDHANT GIRI'),
     (48, 'SONU SETHIA'),
     (49, 'SUNANDA LIMBU'),
     (50, 'SUSHMITA CHHETRI'),
     (51, 'TASHI NAMGYAL BHUTIA'),
     (52, 'TSHERING YANGZOM BHUTIA'),
     (53, 'UTPAL KUMAR CHAUDHARY'),
     (54, 'VICTOR KHADKA'),
     (55, 'ZAMYONG LEPCHA'),
     (56, 'ZEESHAN AHMAD ANSARI'),
     (57, 'DIVYA ROY'),
     (58, 'KAYIZHU PFOZE'),
     (59, 'YISAO PFOZE'),
     (60, '** NO STUDENT **'),
     (61, 'NEHAL GURUNG')]

    session = '2009'
    className = '9A'
    headerType = 1
    allowCondensedMode = 0
    fillInRollNumber = 1
    namesPerColumn = 30

    content =  prepare_input_sheet(className, allRecords, headerType, allowCondensedMode, namesPerColumn, fillInRollNumber, session)

    write_to_file(className + '.txt', content)

[shadwickman]

I don't have time to fully check through all the above code right now, but if you're interested in the guidelines for standardizing Python code, you can have a read through PEP 8.

And as just a speculation, if you're importing this module to use in another program, but don't want certain global variables (as you have opted to use here) accessible by the module it is imported to, then name them with one underscore preceeding their name. Like _array_studentDetails .

[rupeshpradhan]

noted!

BTW, I have gone through the PEP8 and couple of other articles on standardizing python code and have tried to apply them in this module.

It would be really nice if someone could point out some flaws as you did with regards to global variables.

[shadwickman]

Well, there's probably a much easier way of writing the data to the screen in a nice table, as all the conditionals you have in the header_content function don't look too pleasant, even though they get the stuff done.
I wrote a quick table function that accepts a list containing other lists as the indices, and prints the list out into the headings it receives, and makes sure the spacing is kept nice. You could fix it up to fit your script better, or just use it as a template to see how I went about doing it. Maybe if you want, make it spread the data over a fixed-width of 80 characters or something. Anyways, here's the function:

import sys

'''
Display a list of data, where list[0] is a list of headers, and each index after that contains a row of data, corresponding to the order of the headers.
'''
def display_table(matrix, colspace=4, sepchar='-', capheaders=True):

    # set-up variables
    headers = matrix[0]
    data = matrix[1:]
    spacer = ' ' * colspace
    if sepchar:  # if there is a separator
        if len(sepchar) > 1:  # if more than one character
            sepchar = sepchar[0]  # only keep first one
    colwidths = [ 0 for i in range(len(headers)) ]  # init all max lengths to 0
    
    # read lines and store longest values for each column
    for row in matrix:
        for index, item in enumerate(row):
            length = len(str(item))
            if colwidths[index] < length:
                colwidths[index] = length
        
    # write headers
    for index, header in enumerate(headers):
        length = len(str(header))
        padding = ' ' * (colwidths[index] - length)
        header = str(header)
        if capheaders:  # if headers should be uppercase
            header = header.upper()
        sys.stdout.write(str(header) + padding + spacer)
    sys.stdout.write('\n')
        
    # write separator (if applicable)
    if sepchar:
        totLength = 0  # store total width of table
        for length in colwidths:
            totLength += length + len(spacer)
        totLength -= len(spacer)  # rid of length of extra last col spacer
        sys.stdout.write((sepchar * totLength) + '\n')
    
    # write data rows
    for row in data:
        for index, item in enumerate(row):
            length = len(str(item))
            padding = ' ' * (colwidths[index] - length)
            sys.stdout.write(str(item) + padding + spacer)
        sys.stdout.write('\n')
    

##  TEST  ##
tbl = display_table(
    [
        ['Last', 'First', 'Mark'],
        ['Shadwick', 'Mr', 92],
        ['Ulrich', 'Lars', 67],
        ['Keenan', 'Maynard', 83]
    ],
    colspace=5,  # added whitespace between columns
    sepchar=None,  # character used as separator
    capheaders=True  # convert headers to all uppercase
)

You can easily see the structure of the list it takes as the data, with index zero being the headers (from left to right), and each other index being a list of that row's data (again, left to right). The sepchar is what it'll print between the headers and the rows of data (leave as None or "" for it to be disregarded). Sorry if it's not very well commented, and I wrote it without the intent of readability for others, but I hope you can get the gist of what it's doing.
I wrote this a year or so ago and only thought of it again now, so it may appear rough or not too well done, but it worked for me at the time, and I hope it's a good idea for you to implement so that you can just pass headers and data using those conditionals, instead of manually typing spaces, etc out :)