Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm working on some code that deals with mangled C++ function names. I'm displaying these to the user, so to help with readability of some of the really long C++ template class/function names I'm removing some of the template arguments based on how deeply nested the template argument is.

The code below works fine, but it seems fairly clunky. It feels like I'm "writing C in any language". I'm wondering if I can improve it, shorten it, and make it more pythonic.

def removeTemplateArguments(str, depthToRemove):
    """ Remove template arguments deeper than depthToRemove
        so removeTemplateArguments("foo<int>()", 1) gives foo<>()
           removeTemplateArguments("foo< bar<int> >()", 1) gives foo< >()
           removeTemplateArguments("foo< bar<int> >()", 2) gives foo< bar<> >()"""
    if depthToRemove <= 0:
        return str
    currDepth = 0
    res = ""
    for c in str:
        if currDepth < depthToRemove:
            res += c
        if c == "<":
            currDepth += 1
        elif c == ">":
            currDepth -= 1
            if currDepth < depthToRemove:
                res += ">"
    return res
share|improve this question
add comment

2 Answers 2

For that kind of parsing itertools.groupby is the right tool.

With groupby, as I used it down below, every group is either: <,> or an actual piece of code (like foo). So the only thing left to do is to increment/decrement the depth level and decide if the group should be appended or not to the final result list.

from itertools import groupby

def remove_arguments(template, depth):
    res = []
    curr_depth = 0
    for k,g in groupby(template, lambda x: x in ['<', '>']):
        text = ''.join(g)    # rebuild the group as a string
        if text == '<':
            curr_depth += 1

        # it's important to put this part in the middle
        if curr_depth < depth:
            res.append(text)
        elif k and curr_depth == depth:   # add the inner <>
            res.append(text)

        if text == '>':
            curr_depth -= 1

    return ''.join(res)   # rebuild the complete string

It's important to put the depth-level check in between the increment and decrement part, because it has to be decided if the </> are in or out the current depth level.

Output examples:

>>> remove_arguments('foo<int>()', 1)
foo<>()
>>> remove_arguments('foo< bar<int> >()', 1)
foo<>()
>>> remove_arguments('foo< bar<int> >()', 2)
foo< bar<> >()
>>> remove_arguments('foo< bar >()', 2)
foo< bar >()

Also, a couple of quick style notes:

  • Don't use str as variable name or you'll shadow the builitin str.
  • Don't use CamelCase for functions/variable names (look at PEP8).

Inspired by lvc comment, I've grouped here possible lambdas/functions to be used in groupby:

groupby(template, lambda x: x in ['<', '>'])    # most obvious one
groupby(template, lambda x: x in '<>')   # equivalent to the one above
groupby(template, '<>'.__contains__)    # this is ugly ugly
groupby(template, '<>'.count)    # not obvious, but looks sweet

Update

To handle cases like: foo<bar<int>>(), you'll need a better groupby key function. To be specific, a key function that return the current depth-level for a given charachter.

Like this one:

def get_level(ch, level=[0]):
    current = level[0]
    if ch == '<':
        level[0] += 1
    if ch == '>':
        level[0] -= 1
        current = level[0]

    return current

That take advantage of the mutable argument level to perform some kind of memoization.

Observe that remove_arguments will now be more simple:

def remove_arguments(template, depth):
    res = []
    for k,g in groupby(template, get_level):
        if k < depth:
            res.append(''.join(g))
    return ''.join(res)
share|improve this answer
    
You don't need a list in the lambda - x in '<>' does the same. –  lvc Mar 2 '12 at 0:26
    
Template is actually a class from string, we both got that wrong. :) –  Quentin Pradet Mar 2 '12 at 8:12
    
What about something like foo<bar<int>>? Its legal in the new standard (and likely to get used inadvertently regardless) –  Winston Ewert Mar 2 '12 at 15:53
    
@WinstonEwert: Thanks for noticing that. I didn't thought/known of that case, it requires a better groupby key function to handle it. I've updated my answer :) –  Rik Poggi Mar 2 '12 at 20:07
add comment

First of all, your code doesn't seem to work: `removeTemplateArguments("foo< bar >()", 2) has a trailing ">" that shouldn't be here according to your docstring:

  • foo< bar<> >() (expected)
  • foo< bar<> >>() (actual)

You nearly have written a doctest, finishing the work would have helped you catch the bug. Seems not that easy to fix by the way.

Next, I would avoid using names like str which shadows the standard str() function.

My solution relies on greedy regular expressions and recursion (which seems natural here, given the nested template arguments).

import re

def removeTemplateArguments(function_name, depth):
    to_match = "(?P<class>\w+)<\s*(?P<nested>.*)\s*>(?P<paren>\(?\)?)"
    parts = re.match(to_match, function_name).groupdict()
    nested = '' if depthToRemove == 1
      else removeTemplateArguments(parts['nested'], depth-1)

    return "%s<%s>%s" % (parts['class'], nested, parts['paren'])

Not sure how pythonic it is. I do not add any spaces since between chevrons. I could add some logic to add them if needed. I'm using named subgroups to make my regular expression more easy to read.

share|improve this answer
    
thanks for the suggestion and pointing out the bug –  Doug T. Mar 1 '12 at 14:36
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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