15
\$\begingroup\$

This is a simple program I've made using Qt Creator and Pyside:

  • I made a list of activities that I would like to enforce
  • Then I made a program that randomly pick them based on chance
  • It has some buttons to set an activity as Completed/Postponed
  • It can be added new activities for extra 'points'
  • Some activities can take longer than a day, so they are Due to a certain day
  • And finally there's a log in day/week/month with scores and count for each activity

I'm looking for an analysis on my program structure and data flow (this is perhaps my second GUI program, so it probably has many flaws). But I would be very happy to learn how the program feels and what should be added/removed (features). But mostly I'm looking towards doing things properly.

To execute it you need to have these files in the folder:

  • Activities.py
  • day_planner.py
  • day_planner_ui.py
  • due_window_ui.py

Activities.py

import random

class Activity:
    """Set the basic activity properties"""
    def __init__(self, name, occurrence, occur_time, postpone_cost, add_credit, options=None, completion_time=None):
        self.name = name
        self.occurrence = occurrence
        self.occur_time = occur_time
        self.postpone_cost = postpone_cost
        self.add_credit = add_credit
        self.options = options
        self.completion_time = completion_time

    def random_occur(self):
        """randomly chance of this activity occurring in the day"""
        chance = 0
        if self.occur_time == 'Week':
            chance = self.occurrence / 7.0
        elif self.occur_time == 'Month':
            chance = self.occurrence / 30.0

        rand = random.random()
        if rand < chance:
            return self

    def __str__(self):
        return self.name

    def __gt__(self, other):
        return self.name > other.name


activities = {
    "Learn Language": {
        'options': ["French", "German"],
        'occurrence': 1,
        'occur_time': "Week",
        'postpone_cost': 2,
        'add_credit': 1
    },
    "Learn Programming": {
        'options': ["Python", "C", "C#"],
        'occurrence': 1,
        'occur_time': "Week",
        'postpone_cost': 2,
        'add_credit': 1
    },
    "Running/Exercise": {
        'occurrence': 2,
        'occur_time': "Week",
        'postpone_cost': 2,
        'add_credit': 1
    },
    "Clean Room": {
        'occurrence': 1,
        'occur_time': "Week",
        'postpone_cost': 2,
        'add_credit': 1
    },
    "Study": {
        'occurrence': 3,
        'occur_time': "Week",
        'postpone_cost': 2,
        'add_credit': 1
    },
    "Read a Book": {
        'occurrence': 1,
        'occur_time': "Month",
        'postpone_cost': 2,
        'add_credit': 1
    },
    "Meditate": {
        'occurrence': 1,
        'occur_time': "Week",
        'postpone_cost': 2,
        'add_credit': 1
    },
    "Go Out": {
        'occurrence': 1,
        'occur_time': "Month",
        'postpone_cost': 2,
        'add_credit': 1
    },
    "No Social Media for a day": {
        'occurrence': 1,
        'occur_time': "Week",
        'postpone_cost': 2,
        'add_credit': 1
    },
    "No Internet for a Day": {
        'occurrence': 1,
        'occur_time': "Month",
        'postpone_cost': 2,
        'add_credit': 1
    },
    "Drink Water": {
        'occurrence': 2,
        'occur_time': "Day",
        'postpone_cost': 1,
        'add_credit': 0
    },
    "Clean/Organize PC": {
        'occurrence': 1,
        'occur_time': "Month",
        'postpone_cost': 2,
        'add_credit': 1
    },
    "Add a New Item to this list": {
        'occurrence': 1,
        'occur_time': "Month",
        'postpone_cost': 0,
        'add_credit': 0
    },
    "_Dummy Object": {
        'occurrence': 1,
        'occur_time': "Day",
        'postpone_cost': 0,
        'add_credit': 0
    },
    "_Dummy Object 2": {
        'occurrence': 1,
        'occur_time': "Day",
        'completion_time': 1,
        'postpone_cost': 0,
        'add_credit': 0
    }
} 


acts = {name: Activity(name, **activities[name]) for name in activities}

The two _dummy objects are for test purpose: one advances the day, the other is a constant due in 1 day activity to test the Due Window. If you desire to remove them, just comment out them in the Activities.py.

day_planner.py

from day_planner_ui import Ui_Widget as Main_Widget
from due_window_ui import Ui_Widget as Due_Widget
from collections import defaultdict
from PySide.QtGui import *
from PySide import QtCore
from copy import copy
import Activities
import datetime
import calendar
import pickle
import random
import sys

_MONTH_MAP = {m.lower(): i for i, m in enumerate(calendar.month_name[1:])}


# ENUMERATORS
# noinspection PyClassHasNoInit
class Status:
    COMPLETED = "Completed"
    POSTPONED = "Postponed"
    MISSED = "Missed"
    ADDED = "Added"
    SCORE = "Score"

    # for sorting in QTree
    SORT_MAP = {COMPLETED: 4, POSTPONED: 3, MISSED: 2, ADDED: 1, SCORE: 0}


# noinspection PyClassHasNoInit
class Times:
    DAY = "Day"
    WEEK = "Week"
    MONTH = "Month"


# ABSTRACT CLASSES
class Progress:
    """Handle the progress of the day, it's activities, updates and logs to file"""

    def __init__(self):
        self.today = datetime.date.today()
        self.day_activities = []
        self.due_activities = []

        self.completed_activities = []
        self.postponed_activities = []
        self.added_activities = {}

        self.set_activities(Activities.acts)

    def debug_add_day(self):
        """for testing only, add a day to current day and updates"""
        self.today += datetime.timedelta(days=1)
        self.update()

    def update(self):
        """Check if the day have changed and perform updates if it does"""
        today = datetime.date.today()
        if self.today != today:
            day_activities = self.day_activities
            due_activities = self.due_activities
            postponed_activities = self.postponed_activities

            for act in day_activities:
                self.log(act, Status.MISSED)

            for i, act in enumerate(due_activities):
                act.completion_time -= 1
                if act.completion_time == 0:
                    main.display_due_window(act)

            self.set_activities(Activities.acts)

            self.day_activities.extend(postponed_activities)
            self.due_activities.extend(due_activities)

    def log(self, item, status):
        """
        log item with it's status to file
        the format is:
            DATE    SCORE   STATUS  ITEM_NAME
        """
        score = 0
        date = self.today

        if status == Status.ADDED:
            score = item.add_credit
        elif status == Status.POSTPONED:
            score = -item.postpone_cost
        elif status == Status.MISSED:
            score = -item.postpone_cost * 2

        if item.completion_time is not None or status == Status.MISSED:
            # to log in the correct the day
            date -= datetime.timedelta(days=1)

        with open("log_file.txt", 'a') as log_file:
            line = "{}\t{}\t{}\t{}\n".format(date, score, status, item.name)
            log_file.writelines(line)

    def clear_activities(self):
        self.day_activities = []
        self.due_activities = []

        self.completed_activities = []
        self.postponed_activities = []
        self.added_activities = {}

    def set_activities(self, activities):
        """Loops to 'activities' and add them if they meet criteria"""
        self.clear_activities()
        for act in activities:
            act = copy(activities[act])
            if act.occur_time == Times.DAY:
                for i in xrange(act.occurrence):
                    self.add_activity(act)
            else:
                act = act.random_occur()
                if act:
                    self.add_activity(act)

    @staticmethod
    def set_option(activity):
        """Set the selected option if action has one"""
        if activity.options:
            activity.name += " - " + random.choice(activity.options)
            activity.options = None

    def add_activity(self, activity):
        """sets 'activity' options(if exist), and separate between due and daily"""
        self.set_option(activity)

        if activity.completion_time:
            self.due_activities.append(activity)
        else:
            self.day_activities.append(activity)
        return activity

    def remove_activity(self, activity):
        """removes an 'activity' from it's list"""
        try:
            self.day_activities.remove(activity)
        except ValueError:
            self.due_activities.remove(activity)


class TreeData:
    """Set the data structure to be used for the QTreeViews."""
    day = {}
    week = {}
    month = {}

    def __init__(self, name):
        self.name = name
        self.data = {}

    def add_item(self, key, status, item_name, score):
        """
        Sets the structure
                Which consists of a dict with nested defaultdict(int)
                for completed/missed/postponed/added activities and Score
        """
        if self.name != Times.MONTH:
            key = '%s %i' % (self.name, key)

        if key not in self.data:
            self.data[key] = {Status.COMPLETED: defaultdict(int),
                              Status.MISSED: defaultdict(int),
                              Status.POSTPONED: defaultdict(int),
                              Status.ADDED: defaultdict(int),
                              Status.SCORE: 0}

        self.data[key][status][item_name] += 1
        self.data[key][Status.SCORE] += int(score)

    def __iter__(self):
        return iter(self.data)

    def __getitem__(self, item):
        return self.data[item]

    @classmethod
    def setup(cls, main_window):
        """Main method of the class, is used to read and parse the file and set the structure for the QTrees"""
        day_n = 0
        cur_day = None
        week_n = 1
        score = 0

        cls.day = TreeData("Day")
        cls.week = TreeData("Week")
        cls.month = TreeData("Month")

        try:
            with open("log_file.txt") as log_file:
                for line in log_file:
                    # Splits the data into a meaningful way
                    date, score_change, status, item_name = line.strip().split("\t")
                    year, month, day = map(int, date.split("-"))
                    month_name = datetime.date(year, month, day).strftime("%B")

                    # sets the day/week numbers
                    if cur_day != day:
                        cur_day = day
                        day_n += 1
                        if day_n % 7 == 0:
                            week_n += 1

                    # structure the QTrees
                    cls.day.add_item(day_n, status, item_name, score_change)
                    cls.week.add_item(week_n, status, item_name, score_change)
                    cls.month.add_item(month_name, status, item_name, score_change)

                    score += int(score_change)
        except IOError as err:
            print "%s, a file will be created when and action is taken" % err

        main_window.set_score(score)


# GUI CLASSES
class MainFrame(QWidget, Main_Widget):
    """Main Window"""

    def __init__(self):
        QWidget.__init__(self)
        self.setWindowTitle("Day Planner")
        self.setupUi(self)

        # Models
        self.day_tree_model = QStandardItemModel()
        self.week_tree_model = QStandardItemModel()
        self.month_tree_model = QStandardItemModel()

        self.day_tree.setModel(self.day_tree_model)
        self.week_tree.setModel(self.week_tree_model)
        self.month_tree.setModel(self.month_tree_model)

        # clean up
        self.add_warning_label.hide()  # TODO: implement warning when adding unwanted activities
        self.add_window.hide()

        # for pop-windows
        self.due_windows = []

        # set UI items
        self.set_add_items()
        self.set_connections()
        self.refresh()

    # noinspection PyUnresolvedReferences
    def set_connections(self):
        """connects buttons/slots to functions"""
        self.complete_button.clicked.connect(lambda: self.remove_item(Status.COMPLETED))
        self.posptone_button.clicked.connect(lambda: self.remove_item(Status.POSTPONED))

        self.add_button.clicked.connect(self.add_item)
        self.add_window_button.clicked.connect(self.show_add_window)

        self.day_tree.expanded.connect(lambda x: self.expand_children(x, self.day_tree))
        self.week_tree.expanded.connect(lambda x: self.expand_children(x, self.week_tree))
        self.month_tree.expanded.connect(lambda x: self.expand_children(x, self.month_tree))

    def expand_children(self, index, view):
        """when clicking in a QTreeView it expands it's children items"""
        child_count = index.model().rowCount(index)
        for i in xrange(child_count):
            child = index.child(i, 0)
            self.expand_children(child, view)

        if not view.isExpanded(index):
            view.expand(index)

        view.resizeColumnToContents(0)

    def add_item(self):
        """Adds a item to day/due activities"""
        selected = self.add_table.selectedItems()
        if selected:
            item = selected[0].text()
            to_add_item = Activities.acts[item]
            progress.set_option(to_add_item)
            if to_add_item.name not in progress.added_activities:
                progress.added_activities[to_add_item.name] = to_add_item
                added_item = progress.add_activity(to_add_item)
                progress.log(added_item, Status.ADDED)
                self.refresh()

    def remove_item(self, status):
        """Removes a item from day activities"""
        index = self.activivites_list.currentRow()
        if index != -1:
            item = progress.day_activities.pop(index)
            progress.log(item, status)
            if item.name == "_Dummy Object":
                progress.debug_add_day()
            self.refresh()
            self.activivites_list.setCurrentRow(index)

    def show_add_window(self):
        """Pop-up the Add Window"""
        self.add_window.show()
        self.add_window.raise_()

    def set_score(self, score):
        self.score_box.setText(str(score))

    def set_activities(self):
        """Populates the day/due activities lists with items"""
        self.activivites_list.clear()
        for act in progress.day_activities:
            item = QListWidgetItem(act.name)
            self.activivites_list.addItem(item)

        self.due_list.clear()
        for act in progress.due_activities:
            if act.completion_time > 0:
                item = QListWidgetItem("%s due in %d days" % (act.name, act.completion_time))
            else:
                item = QListWidgetItem(act.name)
            self.due_list.addItem(item)

        # Hides the Due list if empty
        if len(progress.due_activities) == 0:
            self.due_list.hide()
            self.due_label.hide()
        else:
            self.due_list.show()
            self.due_label.show()

    def set_add_items(self):
        """Populates the Add Items table with items and they Credits"""
        self.add_table.setRowCount(len(Activities.acts))
        for row, act in enumerate(Activities.acts):
            act = Activities.acts[act]
            item_name = QTableWidgetItem(act.name)
            item_value = QTableWidgetItem(str(act.add_credit))

            self.add_table.setItem(row, 0, item_name)
            self.add_table.setItem(row, 1, item_value)

        self.add_table.sortByColumn(1, QtCore.Qt.DescendingOrder)
        self.add_table.resizeColumnToContents(0)

    def update_trees(self):
        """Populates the QTreeViews with items"""
        TreeData.setup(self)
        self.day_tree_model.clear()
        self.week_tree_model.clear()
        self.month_tree_model.clear()

        self.day_tree_model.setColumnCount(2)
        self.week_tree_model.setColumnCount(2)
        self.month_tree_model.setColumnCount(2)

        self.day_tree.setRootIsDecorated(True)
        self._populate_tree(TreeData.day, self.day_tree_model.invisibleRootItem())
        self._populate_tree(TreeData.week, self.week_tree_model.invisibleRootItem())
        self._populate_tree(TreeData.month, self.month_tree_model.invisibleRootItem())

    @staticmethod
    def _sort_tree(item):
        """Magically sorts 4 different types of data in a single function"""
        try:
            return _MONTH_MAP[item.lower()]
        except KeyError:
            try:
                return int(item.split()[-1])
            except ValueError:
                try:
                    return Status.SORT_MAP[item]
                except KeyError:
                    return item

    def _populate_tree(self, children, parent):
        """Recursively populates the QTreeView with proper sorting"""
        for child in sorted(children, key=self._sort_tree, reverse=True):
            child_item = QStandardItem(str(child))
            row = [child_item]
            if isinstance(children[child], dict):
                self._populate_tree(children[child], child_item)
            else:
                item = QStandardItem(str(children[child]))
                row.append(item)
            parent.appendRow(row)

    def display_due_window(self, item):
        """Pop-ups when a Due activity time is over and prompts for options"""
        due_window = DueFrame(self, item)
        due_window.show()
        due_window.raise_()
        self.due_windows.append(due_window)

    def refresh(self):
        self.set_activities()
        self.update_trees()

    def closeEvent(self, event):
        # noinspection PyShadowingNames
        with open("day_planner.p", 'w') as f:
            pickle.dump(progress, f)


class DueFrame(QWidget, Due_Widget):
    """Pop-up Window to display options for a Due activity which it's time was over"""

    def __init__(self, parent, item):
        """Item is a activity due"""
        QWidget.__init__(self)
        self.setupUi(self)
        self.parent = parent
        self.item = item

        self.resize(self.layout().minimumSize())

        # to handle unwanted close
        self.accept_close = False

        self.due_window_label.setText("The due activity %s time is over" % item.name)

        self.set_connections()

    # noinspection PyUnresolvedReferences
    def set_connections(self):
        self.due_complete_button.clicked.connect(lambda: self.quit_window(Status.COMPLETED))
        self.due_missed_button.clicked.connect(lambda: self.quit_window(Status.MISSED))
        self.due_postponed_button.clicked.connect(lambda: self.quit_window(Status.POSTPONED))

    def quit_window(self, status):
        """Postponed or remove a activity then logs it"""
        self.accept_close = True
        if status == Status.POSTPONED:
            self.item.completion_time += 7
        else:
            progress.remove_activity(self.item)
        progress.log(self.item, status)
        self.parent.refresh()
        self.close()

    def closeEvent(self, event):
        if self.accept_close:
            event.accept()
        else:
            event.ignore()


if __name__ == "__main__":
    try:
        with open("day_planner.p") as f:
            progress = pickle.load(f)
    except (IOError, EOFError) as e:
        print "%s, creating a Progress, save will happen when properly closing the application" % e
        progress = Progress()

    app = QApplication(sys.argv)
    main = MainFrame()
    progress.update()
    main.show()
    sys.exit(app.exec_())

Other required files to run the program. Do not review the code in it, it's auto-generated.

\$\endgroup\$
5
\$\begingroup\$

I think @JanneKarila already raised the most important points, especially critical of those is the global progress variable. I can only add some minor points on top of that. In any case, this is pretty nice code, well done! I suggest to put it up on GitHub so it's easier to clone and play with.

Progress.set_option

This method manipulates an activity: it transforms a generic activity like "Learn Language" with options like "French" or "German" to a specific "Learn Language - German" item.

One problem with this is that the method doesn't belong to Progress, as it is an intimate implementation detail of activity data.

Another problem is that the activity data in Activity.acts is in fact not ready to use, because the option needs to be set. This has the consequence that whenever you want to add an activity, you have to remember to call this method first, for example when you do this in MainFrame.add_item and in Progress.add_activity. This is error prone, as you may forget.

Also due to this issue, Progress.add_activity breaks the single responsibility principle, because it does too things: prepare the activity (by calling set_option) and add the activity.

It would be better to clean this up, by adding a wrapper method in Activities that gets the desired item from the list and sets the option too.

Avoid len(...) == 0

Using len(...) == 0 is not Pythonic. You could make this code Pythonic by flipping the condition and the if-else branches;

if len(progress.due_activities) == 0:
    self.due_list.hide()
    self.due_label.hide()
else:
    self.due_list.show()
    self.due_label.show()

Activity.random_occur

I find it a bit odd for this method to return an activity or None. I think it would be more intuitive to return a boolean, and also slightly simple to write this code:

def random_occur(self):
    # ...
    if rand < chance:
        return self

def set_activities(self, activities):
    # ...
    act = act.random_occur()
    if act:
        self.add_activity(act)

as:

def random_occur(self):
    # ...
    return rand < chance

def set_activities(self, activities):
    # ...
    if act.random_occur():
        self.add_activity(act)

Date hack

To be honest, I didn't really understand the need to hack the date in the log method. It's unexpected and confusing that a log method would alter the date before logging. Maybe you have your reasons, but if possible it would be better to not do this.

Also, I'm wondering if you really need the is not None condition here:

if item.completion_time is not None or status == Status.MISSED:
    # to log in the correct the day
    date -= datetime.timedelta(days=1)

You could just drop the is not None to simplify. The logic will be slightly different for the 0 case, but since you already use if activity.completion_time: in other places, it seems it should be ok.

Avoid debugging code in the public interface

I understand you're using this for testing, for now, but the debug_add_day doesn't belong in the public interface. If you want to test the code works, it's better to write proper unit tests, well separated from the public API.

Use range instead of xrange

You can replace all the xrange calls with range without a harmful effect. It will make your code more compatible with Python 3.

\$\endgroup\$
  • 1
    \$\begingroup\$ Nice review, too bad it was before the bounty was given automatically. I'll probably take most of the suggestions. Feel bad that I still doesn't know how to use GitHub :) Thanks. \$\endgroup\$ – f.rodrigues Feb 4 '15 at 23:39
6
+100
\$\begingroup\$
  • If you want enum types but can't upgrade to Python 3.4, consider using the backport of its standard library enum module.
  • The use of the class method and class attributes in TreeData seems to me a bit like two classes crammed into one. Consider moving the class level stuff to another class, maybe like this:

    class TreeDatas(object):
        def __init__(self):
            self.day = TreeData("Day")
            self.week = TreeData("Week")
            self.month = TreeData("Month")
    
        def setup(self, main_window):
            ...
    
  • On Python 2 remember to make classes inherit from object. Otherwise you get an old-style class with some quirks.

  • The code relies on a global progress variable which is created under the if __name__ == "__main__": guard. The guard makes it possible to import the module into a larger app but the missing global variable would prevent it from working. You could make progress an attribute of MainFrame instead.
\$\endgroup\$
  • \$\begingroup\$ Great, didn't know about the enum module. I'll sure implement it, nice it have a __order__ for sorting. I was just using thus custom enum-like so I didn't have to type "Completed" everywhere, I hate messy strings in the code, it's so easy to spell something wrong, and everything stop working becasue a if status == "Complete" is never trigged. \$\endgroup\$ – f.rodrigues Jan 30 '15 at 1:50

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.