Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

This question already has an answer here:

On a code review, I've stumbled on an interesting idea, which I can't properly judge alone.

Is it OK to improve readability of code by means of not widely known language syntax? When the original writer didn't use a special syntax designated for solving a specific coding problem, and solved it by simulating with workarounds — is it worth to refactor it for clarity and conciseness? What if even the code reviewer didn't know about the language feature?

To be specific, I'll bring the exact issue I've met on the code review — but keep in mind that this is just an example.

So the original code was a little web service in Python. At some point, there arised a requirement for several request handlers to be protected with an authentication mechanism. Somebody solved it with inheritance:

class AuthOnlyAccess:
    def GET(self, *args):
        if not hasattr(self, 'PROTECTED_GET'):
            raise web.Nomethod()

        if check_auth():
            return self.PROTECTED_GET(*args)
        else:
            raise web.Unauthorised()

    def POST(self, *args):
        if not hasattr(self, 'PROTECTED_POST'):
            raise web.Nomethod()

        if check_auth():
            return self.PROTECTED_POST(*args)
        else:
            raise web.Unauthorised()

...

class FooHandler(AuthOnlyAccess):
    def PROTECTED_GET(self, a, b, c):
        handle_foo_get(a, b, c)

    def PROTECTED_POST(self, x, y, z):
        handle_spam_eggs()

...
<more handlers deriving AuthOnlyAccess>

The refactorer found this code ugly and introduced a decorator:

from functools import wraps
...

def requires_auth(wrapped_func):
    """ (a little doc telling that this is a decorator and how to use it) """
    @wraps(wrapped_func)
    def decorator(*args):
        if check_auth():
            return wrapped_func(*args)
        else 
            raise web.Unauthorized()

    return decorator

...

class FooHandler:
    @requires_auth
    def GET(self, a, b, c):
        handle_foo_get(a, b, c)

    @requires_auth
    def POST(self, x, y, z):
        handle_spam_eggs()

...
<more refactored handlers>

The reviewer argues that this change actually decreases maintainability, because most of the team doesn't know decorator syntax. So the next person visiting the refactored code could be puzzled by the construct — but not with the previous version.

There's also a convincing argument that most of the codebase isn't in Python, (the language is there only for that little web service), and hence team members aren't required to have even shallow knowledge of Python. However, to work with code, one's going to need to know its language, right?

And if you have the temptation to say that python decorators aren't that advanced syntax — please remember that this is only an example. Every language has its dark corners. (But to learn those corners means to become better.)

So the question is... Can you read unknown syntax? Is the elegance worth the surprise? At least, what are the tradeoffs and/or guidelines you can name? I do really need some opinions from the community of programming professionals.

share|improve this question

marked as duplicate by Florian Margaine, gnat, MichaelT, GlenH7, Kilian Foth Oct 17 '13 at 11:29

This question has been asked before and already has an answer. If those answers do not fully address your question, please ask a new question.

3  
How can this question be answered except with "it depends?" –  JasonTrue Oct 15 '13 at 21:42
5  
@JasonTrue "it depends on: 1) ... 2) ... 3) ...". –  ulidtko Oct 15 '13 at 21:43
7  
If my team has low skill, should I lower the skill of my code? programmers.stackexchange.com/questions/203469/… talks about closely related issues. –  psr Oct 15 '13 at 22:04
4  
"So the question is... Can you read unknown syntax"? Of course. I'd learned it as needed. Especially for things as standard and well-established as Python decorators. To hear that there are professional programmers who would do otherwise is horrifying in the extreme. –  user16764 Oct 15 '13 at 22:57
7  
"If you think education is expensive, you should try ignorance." - Derek Bok –  Kilian Foth Oct 16 '13 at 6:45

3 Answers 3

up vote 18 down vote accepted

The refactored example is objectively superior because it has less repeated code. Therefore, that implementation is actually simpler, and probably shorter. There is also the argument that the original code in your example is an ad-hoc implementation of traits (or an awkward expression of the strategy pattern), the result being unnecessarily complex code.

In this case, you should be using decorators, even when most of your devs don't know them. This adds only a single extra concept to that piece of code, which is very cheap considering that programming is generally rich in fancy new concepts. The necessary information can and should be added in a terse comment explaining this feature, which should also contain a link to the relevant docs.

For other language features, the same considerations should be made:

  • Which version is simpler and expresses a solution closer to the language of the problem domain?
  • What is the cost of educating future maintainers? Can everything be explained in a short comment, or would you have to hold a lecture on category theory?
  • What is the cost of not educating future maintainers? Just two extra lines here, or having to write baby code to the end of your life?

    Give a man a fish and you feed him for a day.
    Teach a man to fish and feed him for his life.
    Dumb down your code for a programmer and he can understand it at once.
    Teach a programmer advanced techniques, and he'll understand all your future code on his own.

share|improve this answer
    
objectively superior if shorter code is the only metric.. don't forget about stack depth, etc. –  David Cowden Oct 15 '13 at 22:48
4  
@DavidCowden objectively superior under maintainability and correctness metrics like DRY, separation of concerns, etc. which trumps small inefficiencies like an single extra stack frame. –  delnan Oct 16 '13 at 8:19
    
@delnan that's a rather subjective response, sir. infoq.com/presentations/Simple-Made-Easy –  David Cowden Oct 16 '13 at 11:34
1  
@DavidCowden Which part is subjective, that DRY/separation of concerns is more important than stack depth? Because I don't see how adherence DRY and separation and concern can be subjective. –  delnan Oct 16 '13 at 15:21
    
@delnan the general notion that abstraction always trumps efficiency. Although we are on programmers so maybe people operate under that assumption by default. –  David Cowden Oct 16 '13 at 20:07

This is something that you need to take on a case by case basis. I would argue that you should use unfamiliar features of a language if it would improve clarity and make future maintenance easier (both of which your example accomplishes in my opinion).

You should never use features just because you want to be clever or because you want to use the feature for the sake of it. On the other hand, I don't think you should ever avoid using a feature that would improve the code simply because the other developers on your team don't know how to use it yet. If they are worth their salt they will soon learn and use it themselves if the idea has merit. Developers that refuse to adopt possibly superior techniques just because they've never seen or used them before should raise big red flags.

share|improve this answer

First of all, I would say that coding conventions are a place where restrictions on language usage are stored. Otherwise every time people will have different idea about what is familiar for a team and what isn't.

When choosing a subset you create a new language. It certainly shares many things with "host" language, but is different. Some things may become simpler, some - harder.

Why define subset:

  1. Host language has features considered dangerous (e.g. limiting preprocessor in C++)
  2. Your needs are much smaller than language abilities (e.g. JSON as a subset of JavaScript)
  3. You have specific needs (e.g. supporting old versions of a compiler without support for the newest language features)

Why stick to the whole language:

  1. The new features are there for a reason (e.g. your case)
  2. Better support ("how to solve it in LISP?" will be answered faster than "how to solve this in LISP without lambda functions")
  3. Hiring new developers simpler (for subset the candidates should be able and comfortable to work under restrictions you have)
  4. You trust that the whole language was designed by better language designer(s) than you :)
share|improve this answer

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