7
\$\begingroup\$

See the proposed changes in this Pull Request under def add_talk.

date = self.talkDetailsWidget.dateEdit.date()
time = self.talkDetailsWidget.timeEdit.time()
presentation = Presentation(
    unicode(self.talkDetailsWidget.titleLineEdit.text()).strip(),
    unicode(self.talkDetailsWidget.presenterLineEdit.text()).strip(),
    unicode(self.talkDetailsWidget.descriptionTextEdit.toPlainText()).strip(),
    unicode(self.talkDetailsWidget.categoryLineEdit.text()).strip(),
    unicode(self.talkDetailsWidget.eventLineEdit.text()).strip(),
    unicode(self.talkDetailsWidget.roomLineEdit.text()).strip(),
    unicode(date.toString(Qt.ISODate)),
    unicode(time.toString(Qt.ISODate)))

There's a lot of boilerplate code (e.g. unicode(), seld.talkDetailsWidget, text(), strip(), etc.) How could you reduce that and still have the code be easy to understand?

My thinking is if something along the lines of this were possible:

map(str.strip,
    map(unicode, 
        map(QLineEdit.text,
            map(self.talkDetailsWidget, fields))))
\$\endgroup\$

closed as off-topic by Stephen Rauch, Billal Begueradj, Toby Speight, Graipher, Daniel Jul 9 '18 at 13:29

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Lacks concrete context: Code Review requires concrete code from a project, with sufficient context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site." – Stephen Rauch, Billal Begueradj, Toby Speight, Daniel
If this question can be reworded to fit the rules in the help center, please edit the question.

  • 2
    \$\begingroup\$ Had you used Python 3.4, you could have used functools.singledispatch to create an overload of Presentation.__init__ taking a TalkDetailsWidget parameter. \$\endgroup\$ – Morwenn Mar 25 '14 at 10:27
  • \$\begingroup\$ @Morwenn that's good to know! We want to move over to Python3, but like most other projects, we need our dependencies to be compatible first. We should look into it again, maybe we're much closer now than the last time we checked. \$\endgroup\$ – Dennis Mar 25 '14 at 13:18
  • \$\begingroup\$ Moving to Python 3 can be pretty tricky, but it's worth. I would say that the biggest problem is the current lack of some libraries (I'm looking at you, PIL). \$\endgroup\$ – Morwenn Mar 25 '14 at 13:25
8
\$\begingroup\$

Your TalkDetailsWidget is underdeveloped, I think. You could say that you have a view but no model, and that is causing you problems.

You want to be able to write

talk = self.talkDetailsWidget
presentation = Presentation(title=talk.title,
                          speaker=talk.presenter,   # ← Why the inconsistent vocabulary?
                      description=talk.description,
                         category=talk.category,
                            event=talk.event,
                             room=talk.room,
                             date=unicode(talk.date.toString(Qt.ISODate)),
                             time=unicode(talk.time.toString(Qt.ISOTime)))

Therefore, you'll need to implement new properties in TalkDetailsWidget. To avoid copy-and-paste programming in TalkDetailsWidget, I suggest writing those getters using metaprogramming.

class TalkDetailsWidget(QWidget):
    …

    def _field_reader(field, method='text'):
        return property(fget=lambda self: unicode(getattr(getattr(self, field), method)()).strip(),
                        doc="Returns a Unicode string from the %s field with spaces stripped from each end" % (field))

    title       = _field_reader('titleLineEdit')
    presenter   = _field_reader('presenterLineEdit')
    description = _field_reader('descriptionTextEdit', method='toPlainText')
    category    = _field_reader('categoryLineEdit')
    event       = _field_reader('eventLineEdit')
    room        = _field_reader('roomLineEdit')

    @property
    def date(self):
        self.dateEdit.date()

    @property
    def time(self):
        self.timeEdit.time()
\$\endgroup\$
  • \$\begingroup\$ Unrelated note - your code violates PEP8 with aligning lines and 80 characters per columns limit. \$\endgroup\$ – Ruslan Osipov Mar 25 '14 at 19:19
  • \$\begingroup\$ @RuslanOsipov I am aware that it violates PEP8. The fget parameter could use a proper function instead of a lambda. Not sure what to do about the docstring. As for the unusual alignment by the equals sign for the arguments of Presentation, I think the violation aids readability. \$\endgroup\$ – 200_success Mar 25 '14 at 19:23
  • \$\begingroup\$ Makes sense. The docstring can be wrapped in ("line 1"\n"line 2"), but it's not important in the context. \$\endgroup\$ – Ruslan Osipov Mar 25 '14 at 19:57

Not the answer you're looking for? Browse other questions tagged or ask your own question.