Lessons from a month with Martin

Puneeth Chaganti

puneeth@enthought.com

2013-11-6

Table of Contents

Code is Communication

Programs must be written for people to read, and only incidentally for machines to execute – Hal Abelson, SICP

  • Represent the current best knowledge of your problem
  • Make it obvious!
    • Create intent in your code
    • Features should be easy to explain to the user
  • Layer your code, with a language at each layer
  • Developers need a language to communicate in!

Represent the best knowledge of the Problem

  • To build something, you only need to know two things:
    • What you are building
      • Have a Coherent story!
    • How to build it
      • Software engineering, and all that jazz…
  • Anything on the drawing board should appear in your code!
  • Refactor to accurately represent the current best knowledge!
    • We are refactorers, not programmers
  • Remove unused code

No murder mysteries

def main():
    def application_factory(*args, **kw):
        from canopy.app.application import CanopyApplication
        return CanopyApplication(*args, **kw)

    from canopy.app.application_launcher import ApplicationLauncher
    launcher = ApplicationLauncher(application_factory=application_factory)
    launcher.launch()
def main():
    from canopy.app.application import CanopyApplication
    from canopy.app.application_launcher import ApplicationLauncher
    launcher = ApplicationLauncher(application_factory=CanopyApplication)
    launcher.launch()
def main():
    def application_factory(*args, **kw):
        """ Delaying GUI imports that require an X server. """
        from canopy.app.application import CanopyApplication
        return CanopyApplication(*args, **kw)

    from canopy.app.application_launcher import ApplicationLauncher
    launcher = ApplicationLauncher(application_factory=application_factory)
    launcher.launch()

Naming

  • Naming is everything!
  • It greatly helps clarify the intent
  • Use the same name, for the same thing!
launcher = ApplicationLauncher(application_factory=app_factory)
-    def create_venv(self, location):
+    def create_venv(self, prefix):

...

         environment = self.__class__(
             base_prefix = self.base_prefix,
-            prefix      = location
+            prefix      = prefix
         )

Layering

  • Layer your code!
  • Each layer should have a DSL of it's own!
  • (Helps rating code for criticality…)
if nuclear_reactor.is_over_heating():
    nuclear_reactor.shutdown()
if nuclear_reactor.temperature > MAX_TEMP * 0.90:
    board = nuclear_reactor.control_board
    control = board.get_controller(id='shutdown')
    send_high(control.pin['8'])

Layering with Envisage

2 steps to building extensible applications with Envisage:

  1. Design/build your application just as you would without Envisage
  2. Write small pieces of code to make your application extensible with Envisage

Smells

  • A plugin trying to find another plugin
  • Importing application code at the module-level of a plugin
  • Looking up a service by name
  • Creating Envisage objects to test application domain objects
  • Lots of code in a plugin/task/task-pane

General guidelines for better functions/methods

These are general rules of thumb, to help you detect smell!

  • Longer than 10 lines or so
  • Too many usages of a single keyword
  • Ugliness from 5 paces away
  • Block comments, splitting code into separate blocks
  • Does more than what it says on the can!

These guidelines, will help you write functions that do one thing (and hopefully, do it well!)

Keep them short: 10 lines or so…

 def __init__(self, plugins=None, **traits):

     super(Application, self).__init__(**traits)

     self._initialize_application_home()
     set_default_preferences(self.preferences)
     if plugins is not None:
         map(self.add_plugin, plugins)

     return

def _initialize_application_home(self):
    """ Initialize the application home directory. """

    ETSConfig.application_home = os.path.join(
        ETSConfig.application_data, self.id
    )

    # Make sure it exists!
    if not os.path.exists(ETSConfig.application_home):
        os.makedirs(ETSConfig.application_home)

    return

one usage of a keyword per function

A rule of thumb, to keep them short!

def _execute(self, options, args):
    """Create a new post or page."""
    ...
    if len(args) > 1:
        print(self.help())
        return False
    elif args:
        path = args[0]
    else:
        path = None
    ...
    if twofile:
        onefile = False
    if not onefile and not twofile:
        onefile = self.site.config.get('ONE_FILE_POSTS', True)
    ...
    if not path:
        txt_path = os.path.join(output_path, slug + suffix)
    else:
        txt_path = path
    ...

Turn comments into doc strings

def real_scan_files(site):
    task_fnames = set([])
    real_fnames = set([])
    output_folder = site.config['OUTPUT_FOLDER']

    # First check that all targets are generated in the right places
    for task in os.popen('nikola list --all', 'r').readlines():
        task = task.strip()
        if output_folder in task and ':' in task:
            fname = task.split(':', 1)[-1]
            task_fnames.add(fname)

    # And now check that there are no non-target files
    for root, dirs, files in os.walk(output_folder):
        for src_name in files:
            fname = os.path.join(root, src_name)
            real_fnames.add(fname)

    only_on_output = list(real_fnames - task_fnames)

    only_on_input = list(task_fnames - real_fnames)

    return (only_on_output, only_on_input)
def list_target_files(target_dir):
    """ List all the output files to be generated from all the tasks. """

    task_fnames = set([])
    for task in os.popen('nikola list --all', 'r').readlines():
        task = task.strip()
        if target_dir in task and ':' in task:
            fname = task.split(':', 1)[-1]
            task_fnames.add(fname)

    return task_names

def list_existing_files(output_dir):
    """ Return a set of paths of all existing files in the given directory. """

    real_fnames = set([])
    for root, dirs, files in os.walk(output_dir):
        for src_name in files:
            fname = os.path.join(root, src_name)
            real_fnames.add(fname)

    return real_fnames

def real_scan_files(site):
    """ Return a tuple of extra files, missing files in the output directory. """

    output_folder = site.config['OUTPUT_FOLDER']
    task_fnames = list_target_files(output_folder)
    real_fnames = list_existing_files(output_folder)

    only_on_output = list(real_fnames - task_fnames)
    only_on_input = list(task_fnames - real_fnames)

    return only_on_output, only_on_input

The 5 paces view

Hint: If it doesn't look pretty from so far, it may be smelly!

def _execute(self, options, args):
    """Create a new post or page."""
    compiler_names = [p.name for p in
                      self.site.plugin_manager.getPluginsOfCategory(
                          "PageCompiler")]

    if len(args) > 1:
        print(self.help())
        return False
    elif args:
        path = args[0]
    else:
        path = None

    is_page = options.get('is_page', False)
    is_post = not is_page
    title = options['title'] or None
    tags = options['tags']
    onefile = options['onefile']
    twofile = options['twofile']

    if twofile:
        onefile = False
    if not onefile and not twofile:
        onefile = self.site.config.get('ONE_FILE_POSTS', True)

    post_format = options['post_format']

    if not post_format:  # Issue #400
        post_format = get_default_compiler(
            is_post,
            self.site.config['COMPILERS'],
            self.site.config['post_pages'])

    if post_format not in compiler_names:
        LOGGER.error("Unknown post format " + post_format)
        return
    compiler_plugin = self.site.plugin_manager.getPluginByName(
        post_format, "PageCompiler").plugin_object

    # Guess where we should put this
    entry = filter_post_pages(post_format, is_post,
                              self.site.config['COMPILERS'],
                              self.site.config['post_pages'])

    print("Creating New Post")
    print("-----------------\n")
    if title is None:
        print("Enter title: ", end='')
        # WHY, PYTHON3???? WHY?
        sys.stdout.flush()
        title = sys.stdin.readline()
    else:
        print("Title:", title)
    if isinstance(title, utils.bytes_str):
        title = title.decode(sys.stdin.encoding)
    title = title.strip()
    if not path:
        slug = utils.slugify(title)
    else:
        if isinstance(path, utils.bytes_str):
            path = path.decode(sys.stdin.encoding)
        slug = utils.slugify(os.path.splitext(os.path.basename(path))[0])
    # Calculate the date to use for the post
    schedule = options['schedule'] or self.site.config['SCHEDULE_ALL']
    rule = self.site.config['SCHEDULE_RULE']
    force_today = self.site.config['SCHEDULE_FORCE_TODAY']
    self.site.scan_posts()
    timeline = self.site.timeline
    last_date = None if not timeline else timeline[0].date
    date = get_date(schedule, rule, last_date, force_today)
    data = [title, slug, date, tags]
    output_path = os.path.dirname(entry[0])
    meta_path = os.path.join(output_path, slug + ".meta")
    pattern = os.path.basename(entry[0])
    suffix = pattern[1:]
    if not path:
        txt_path = os.path.join(output_path, slug + suffix)
    else:
        txt_path = path

    if (not onefile and os.path.isfile(meta_path)) or \
            os.path.isfile(txt_path):
        LOGGER.error("The title already exists!")
        exit()

    d_name = os.path.dirname(txt_path)
    utils.makedirs(d_name)
    metadata = self.site.config['ADDITIONAL_METADATA']
    compiler_plugin.create_post(
        txt_path, onefile, title=title,
        slug=slug, date=date, tags=tags, **metadata)

    event = dict(path=txt_path)

    if not onefile:  # write metadata file
        with codecs.open(meta_path, "wb+", "utf8") as fd:
            fd.write('\n'.join(data))
        with codecs.open(txt_path, "wb+", "utf8") as fd:
            fd.write("Write your post here.")
        LOGGER.notice("Your post's metadata is at: {0}".format(meta_path))
        event['meta_path'] = meta_path
    LOGGER.notice("Your post's text is at: {0}".format(txt_path))

    signal('new_post').send(self, **event)

Guidelines for modules

  • Too many imports, is a sign that your module is probably doing too much!
  • one class per module, ideally
  • Import as close to where we use, rather than at the top

Model/View

  • Keep them separate, obviously!
  • Views should be able to run by themselves
    • add debug code in __main__

Communicate with co-developers/users

  • Good code is rarely written by a developer locked in a room
  • Have more discussions about the code you are writing
  • It helps to have a user driving your API
  • Improves clarity of the ideas in your head

Poised development

  • Sprints! seriously?
  • We follow agile development practices, we don't have a roadmap! – somewhere on the internet
  • Don't add yet another "for-now" hack. Refactor until fixing the bug/adding the new feature is simple/trivial
    • Write a test
    • Get to Green, ASAP
    • Remove duplication!
  • Time to hack it up != estimate for adding the feature
    • Testing
    • Refactoring
    • Documentation
  • Move FIXMEs out of the code to your bug tracker
    • They become the top priority items in your next release cycle