I have a method for generating a report in Python. I want to organize the section about the date into a separate method so I can repeat the report for different dates.

What's the best way to organize this? Should I create a method using just *args or **kwargs where I can dump in all the various dicts...etc.? Does it make sense to be creating all these dicts in the first place?

Here's a link to the other modules.

The whole thing just feels messy and I'm not sure how or where to break it down into smaller chunks.

import sys
import os
from collection import Coll # a module I wrote
import pymongo
from pymongo import MongoClient
from datetime import datetime as dt 
from agents import Agents # a module I wrote
from user import Users # a module I wrote
from propertie import Propertie # a module I wrote
from emails import Emails # a module I wrote
from bson.objectid import ObjectId
import time
import datetime
from tabulate import tabulate
import collections 
import pdb

class Report(object):
    """creat the report from all the data"""
    def __init__(self, database_name, db_collections):
        self.database_name = database_name
        self.db_collections = db_collections

     def RcreateReport(self):
        agents = Agents(self.database_name) #agent class in agent module
        user = Users(self.database_name) # collections_dict['user']
        propertie = Propertie(self.database_name) #collections_dict['property']
        emails = Emails(self.database_name)

        agent_coll_obj      = agents.returnCollection()
        user_coll_obj       = user.returnCollection()
        propertie_coll_obj  = propertie.returnCollection()
        email_coll_obj      = emails.returnCollection()

#### this next section, I think, should be organized into a separate method
#### as the code is repeated below using dates.
        user_id_email_dict                  = user.CidToEmailD()  # dict of all user ids and emails
        user_email_id_dict                  = user.CemailToIdD()  # dict of all user emails and ids
        pure_user_dict                      = user.UpureUserD() # dict of users who are not agents id:email
        pure_users_with_agents_dict         = user.UuserWithAgentD() # dict of pure user Id: agent_id
        user_id_invite_dict                 = user.UuserInviteCountD(pure_users_with_agents_dict, \
                                                                 user_id_email_dict, \
                                                                 email_coll_obj)  # dict of user_id: invite count
        ruurl_set                           = user.UruurlS(pure_users_with_agents_dict, \
                                                       user_id_invite_dict)  # set of user ids'  - implied ruur

        # dict of user_email:agent_id - just users wtih agents                      
        user_email_agent_id                 = user.UuserEmailAgentIdD(user_email_id_dict, \
        # dic of user_id and email_count
        user_id_email_count                 = user.UuserIdemailCountD(email_coll_obj, user_email_agent_id, user_email_id_dict)

        # get a dict of agents : email_count -- agentsGtZero are agents with email count > 0
        agentUserEmailCount, agentsGtZero   = agents.AagentUserEmailTotalD(user_id_email_count,agentUserIdDict)

        # list of agents with 'build_profile' = 0
        agents_build_zeroL                  = agents.dateQuery({"build_profile":0})

        # dates based queries
        start_yr    = 2015
        start_mnth  = 2
        start_day   = 1
        end_yr      = 2015
        end_mnth    = 2
        end_day     = 28

        #created at string to be used as an augment in methods where need to add date query strings
        created_at_string = { "created_at": {"$gte" : dt(start_yr, start_mnth, start_day),"$lt" : dt(end_yr, end_mnth, end_day)}}

 ### repeated code begins here ###
        # total agents
        total_agents_date               = agents.dateQuery(strtYr = start_yr, strtMth = start_mnth, strtDy = start_day,\
                                         endYr = end_yr, endMth = end_mnth, endDy = end_day)
        # total pure users
        total_users_date                = user.dateQuery(query_string = {"is_agent":{"$ne":True}}, strtYr = start_yr, \
                                                                  strtMth = start_mnth, strtDy = start_day,\
                                                                  endYr = end_yr, endMth = end_mnth, endDy = end_day)

        # dict of user_id and email_count With date
        user_id_email_count_date        = user.UuserIdemailCountD(email_coll_obj, user_email_agent_id, user_email_id_dict, created_at_string)

         users_with_agent_date          = user.UuserWithAgentD(query_date = created_at_string)

        # users with invites  - withing dates
        users_with_invites_date         = user.UuserInviteCountD(pure_users_with_agents_dict, \
                                                     user_id_email_dict, \
                                                     email_coll_obj, \
                                                     created_at_string)  # dict of user_id: invite count

        ruurl_set_date                  = user.UruurlS(users_with_agent_date, \
                                               users_with_invites_date) # set of user ids'  - implied ruur

        agents_build_zeroL_date         = agents.dateQuery(query_string = {"build_profile":0}, strtYr = start_yr, \
                                                                  strtMth = start_mnth, strtDy = start_day,\
                                                                  endYr = end_yr, endMth = end_mnth, endDy = end_day)

        # get a dict of agents : email_count -- agentsGtZero are agents with email count > 0

        agentUserEmailCount_date, agentsGtZero_date     = agents.AagentUserEmailTotalD(user_id_email_count_date,agentUserIdDict)

        # begin writing the info to a new file
        ts = time.time()
        st = datetime.datetime.fromtimestamp(ts).strftime('%Y-%m-%d %H:%M:%S')
        f = open("report.txt", "w")
        # report title
        title_table = [("HomeKeepr Report", st)]
        # self.write_line(f, "HomeKeepr Report:", st)
        # first table of totals
        first_table = [
                    ("Total agents",                            len(agentUserIdDict)),
                    ("Total users",                             len(pure_user_dict)),
                    ("Users with agent",                        len(pure_users_with_agents_dict)),
                    ("Users with agent, email:agent_id_dict" ,  len(user_email_agent_id)),
                    ("Users with invites",                      len(user_id_invite_dict)),
                    ("RUURL accounts",                          len(ruurl_set)),
                    ("Agents: build_profile = 0",               len(agents_build_zeroL)),
                    ("Agents, email count > 0",                 len(agentsGtZero))

        f.write(tabulate(first_table, headers = ["Category", "Total"]))
        outputList = dictOfOutputs.items()
        agent_table = outputList
        # print tabulate(agent_table)
        f.write(tabulate(agent_table, headers = ["Number of Clients", "Number of Agents"]))
        dates_table = [("Start Date", start_yr, start_mnth, start_day), ("End Date", end_yr, end_mnth, end_day)]
        f.write(tabulate(dates_table, headers = ["Year", "Month", "Day"]))
        date_table = [
                ("Total agents: created_at",                len(total_agents_date)),
                ("Total users: created_at",                 len(total_users_date)),
                ("Users with agent: created_at",            len(users_with_agent_date)),
                ("Users with invites",                      len(users_with_invites_date)),
                ("RUURL accounts",                          len(ruurl_set_date)),
                ("Agents: build_profile = 0",               len(agents_build_zeroL_date)),
                ("Agents, email count >0",                  len(agentsGtZero_date))

        f.write(tabulate(date_table, headers = ["Category", "Total"]))

  • 3
    \$\begingroup\$ Stop writing classes! \$\endgroup\$ – Gareth Rees May 27 '15 at 13:32
  • \$\begingroup\$ Do you have the source files for your modules, collection, agents, user, propertie, and emails? There may be some user who'd like the background logic as well. Thanks! \$\endgroup\$ – Ethan Bierlein May 27 '15 at 13:35
  • \$\begingroup\$ @GarethRees I'm self taught with no CS background. Can you expand a bit on your answer? I though Classes and creating objects was a desirable way to organize code. Would love to know where I'm going wrong. \$\endgroup\$ – dwstein May 27 '15 at 13:36
  • \$\begingroup\$ @EthanBierlein I do. Is it appropriate to post all that code here? I'd love to get it all reviewed. \$\endgroup\$ – dwstein May 27 '15 at 13:37
  • \$\begingroup\$ @dwstein You don't necessarily have to post your code here, just leave the code you want to get reviewed, and post a link to your background logic on Github, or whatever you use. \$\endgroup\$ – Ethan Bierlein May 27 '15 at 13:38

Follow standard formatting

It is common practice to put standard modules first and then personal or non standard ones, like:

import sys
import os
from datetime import datetime as dt
import time
import datetime
import collections

import pymongo
from pymongo import MongoClient
from tabulate import tabulate
from bson.objectid import ObjectId
import pdb

from collection import Coll  
from agents import Agents  
from user import Users  
from propertie import Propertie  
from emails import Emails  

Where the first block is Standard, the second one is easily downloadable and the third is your own.

You use huge amounts of spaces (e.x. user_id_email_dict = user.CidToEmailD() # dict of all user ids and emails that are usually avoided in Python.

Stop writing classes

In Python functions can float freely around in the global namespace, classes should be used only to combine data-structures with data.

Close automatically

It is nice that you remembered f.close() but you could easily have forgotten it, instead I suggest using with to handle close automatically.

Try and use named constants

For example the filename "report.txt" is arbitrary, I would define a top module constant REPORT_FILE = report.txt".

And yes you should use smaller functions

Try and extract the smaller pieces of functionality inside theyr own function, you only can do it as you are the one with the most experience with this code.


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.