157

For example, there is a common snippet in JS to get a default value:

function f(x) {
    x = x || 'default_value';
}

This kind of snippet is not easily understood by all the members of my team, their JS level being low.

Should I not use this trick then? It makes the code less readable by peers, but more readable than the following according to any JS dev:

function f(x) {
    if (!x) {
        x = 'default_value';
    }
}

Sure, if I use this trick and a colleague sees it, then they can learn something. But the case is often that they see this as "trying to be clever".

So, should I lower the level of my code if my teammates have a lower level than me?

15
  • 43
    I believe this boils down to the question of "Should you write idiomatic code and force people to that level? or write non-idiomatic code that spells out everything explicitly?"
    – user40980
    Commented Jul 2, 2013 at 20:54
  • 53
    Don't lower the skill level of your code! I learnt so much from reading the code of more advanced programmers. Create a culture where, if your peers don't understand something, they're encouraged to ask (and learn). Just make sure you're consistent. Commented Jul 3, 2013 at 11:56
  • 6
    Do you have code reviews? That would be an excellent place for them to ask questions about code like that.
    – thegrinner
    Commented Jul 3, 2013 at 13:16
  • 3
    Part of coding skill is clarity, taking into consideration your "audience." This particular idiom seems worth teaching, but there will certainly be cases where it will make more sense to use a more transparent coding style.
    – LarsH
    Commented Jul 3, 2013 at 14:20
  • 3
    Is it just mean who thinks the second example is of better quality then the first example since what is being done is crystal clear? Seems like the second example is more readable then the short handed version that is the first exmaple. Are there not tools that will automatically take human created code an optimize it for Javascript? Based on my Javascript experience the actually code that is ran doesn't have to be pretty just run as effective as possible.
    – Ramhound
    Commented Jul 3, 2013 at 14:55

13 Answers 13

136

Ok, here goes my take on this big and complicated topic.


Pros for keeping your coding style:

  • Things like x = x || 10 are idiomatic in JavaScript development and offer a form of consistency between your code and the code of external resources you use.
  • Higher level of code is often more expressive, you know what you get and it's easier to read across highly trained professionals.
  • You'll enjoy your job more. I personally value creating pretty code. I think it brings me a lot of satisfaction in my work.
  • Generally it creates more readable style. Sticking with the idioms of the language can be very valuable - they are often idioms for a reason.

Cons for keeping your coding style:

  • It'll be harder for the lower-level programmers to keep up. These are often the people maintaining your code and the ones who'll have to actually read the stuff you write.
  • Maintainers of code, often JavaScript code come from other languages. Your programmers might be competent in Java or C# but not understand how and when JavaScript differs exactly. These points are often idiomatic - an immediately invoked function expression (IIFE) is an example of such a construct.

My personal opinion

You should not lower the skill of your code. You should aspire to write code that is expressive, clear and concise. If you have any doubts about the level of your team - educate them. People are more than willing to learn than you might think, and are willing to adapt new constructs when convinced they are better.

If they think you are 'just being clever,' try to argue your point. Be willing to admit that you're wrong sometimes, and no matter what, try to keep styles consistent throughout your work environment. Doing so will help to avoid hostility.

The most important thing is to stay consistent.

A team's code should be written as if one person coded it. You absolutely have to agree on coding guidelines. You should abide by those guidelines. If the coding guidelines specify that reading optional parameters should be done in the 'less clever' way, then that is the way.

14
  • 1
    I know learning is a constant thing, but is it really this developer's job to train his fellow employees? It really should be management's job to find the most appropriate training for them.
    – corsiKa
    Commented Jul 3, 2013 at 14:38
  • 8
    @corsiKa Those developers are unlikely to learn all the idiosyncrasies of a language through "paid training" (i.e. the kind of training management would send staff to). I consider it a diseased workplace when coworkers don't learn from each other. It's not like the OP would have to give them classroom training. People can just ask questions when they're stuck, and as mentioned in a comment above, code reviews can be good for sharing that kind of knowledge. Commented Jul 3, 2013 at 17:40
  • 2
    His fellow employees should be learning on their own, from the examples set by the OP (and others). Otherwise, they will never progress beyond their current knowledge. The OP shouldn't have to be taking hours out of every day to train them personally, but code reviews and an occasional brown-bag session can help everyone (well, everyone who wants to learn, that is).
    – alroc
    Commented Jul 3, 2013 at 17:40
  • 1
    @corsiKa agreed that a review of a senior dev's code is not adequate training by itself, though it can be a good way to identify things the junior dev should go look up later.
    – Dan Lyons
    Commented Jul 3, 2013 at 18:45
  • 2
    @yms Fair enough, that's a valid standpoint. I never argued that readability is king. I have a strong objection to using multiple languages as if they were the same language. An objection 'earned' in hundreds of hours of debugging. While I completely agree that readability is king, I believe you can't treat codes in several languages similarly. Moreover, I think most of the 'bad reputation' JavaScript has is because of exactly that. People expect it to behave like some-other-language but it doesn't. I agree that readability is mission critical. Better code is always more readable :) Commented Jul 4, 2013 at 19:53
47

Comment Well

Should you lower the skill of your code? Not necessarily, but you should definitely raise the skill of your comments. Be sure to include good comments in your code, especially around the sections you think might be more complicated. Don't use so many comments that the code becomes hard to follow, but be sure to make the purpose of each section clear.

The reality is that being slightly more verbose with comments can be useful with less skilled team members, but those with the lowest skill with ignore them, especially if there's too many, so don't overdo it.

A Matter of Style?

The example you provided is somewhat basic, but also rather stylistic. A comment around every variable default would be pretty tedious to maintain and read. Instead, stylistic or repeated shortcuts or code patterns should probably be established as a standard. If you think something like that form of parameter defaulting should be understood by all and used every time, write these ideas down and take them to your team lead. It's possible all it will take to teach your teammates is a simple meeting where you discuss the standards you proposed.

As another answer already stated, keep it consistent.

Teach a Man To Fish...

Teaching your teammates is probably the best way you can help everyone involved. Make it clear that if anyone has a question over a piece of code with your name in the commit log or timestamps they should feel free to ask you about it. If your team has code reviews, this is a great opportunity to explain any possibly confusing (ahem) well-commented code to your teammates. If your team does not have code reviews, why not? Get to it!

You need to be careful, though. You might not always be around to teach people and you may even forget what you were originally trying to do in a given section of code.

"Clever" Tricks

Keeping the abilities of your teammates in mind is definitely important, but writing maintainable code often means not using arcane shortcuts to problems which might have more common solutions. This is important even when your teammates are intelligent. You do not want to make the code take too long to grasp or to have subtle but important side effects that could be missed. In general, it's best to avoid "clever" tricks when there are suitable alternatives. You never know who might have to maintain the code down the line - often times the older versions of ourselves will not remember the details or reasons for these tricks.

If you find that you must implement a clever trick, at least follow the next bit of advice...

KISS

When in doubt, keep it simple. Whether code is simple or not doesn't necessarily correspond to programmer skill like you might think. In fact some of the most brilliant solutions to a problem are the simplest ones, and some of the more complicated solutions end up on TheDailyWTF. Keeping your code simple and concise can make some of the more intelligent but possibly counter-intuitive decisions easier to grasp.

7
  • 10
    The issue is that language features are seen as "clever tricks", even when I think they clearly aren't. Ever seen a closure? Ever seen an IIFE? Ever seen a function reference passed as a callback? Those are language features that every experienced JS dev knows. Yet they're "clever tricks" to less-experienced JS devs. Commented Jul 3, 2013 at 8:21
  • 1
    @FlorianMargaine sounds to me like you need to work on changing the terminology, ie: these aren't "clever tricks", these are more advanced features of the language... 1 implies your code is not readily understood/a bad thing, 2 implies an opportunity to learn and improve 'my' coding skills (how to get others to change their terminology? Comments, encourage questions, share code articles explaining how these features are useful, etc...) Commented Jul 3, 2013 at 11:19
  • 3
    If it eases any of your headache, part of the frustration may be that Javascript as a language...doesn't make sense. It makes sense to US, the people posting here, because we've done it a long time. But no matter how we've made the language to work "well", to the neutral eye it just doesn't make sense. On another note; you may, sadly, be demonstrating the value of hiring experienced devs, or preferably, 'any-language' developers with a high willingness to learn new paradigms.
    – Katana314
    Commented Jul 3, 2013 at 13:16
  • 1
    @FlorianMargaine I work primarily in JavaScript as well, I know the pain you're feeling. I've approached this by trying to educate teammates. The Crockford JavaScript videos (1, 2) help. I don't think those things you listed fall under "clever" tricks- your teammates should learn them- but some of the things you do with those language features could be the bad kind of "clever". How to convince your company to hire experienced Devs is probably another question entirely...
    – Corion
    Commented Jul 3, 2013 at 14:48
  • 2
    Comments aren't always good. I read "Clean Code" a while back and some points he makes about comments are excellent. If you feel the need to write comments to explain your code, there's a good chance that the code is badly written. If the code was more expressive, a comment is superfluous. Every time you're about to write a comment, stop for a moment to consider if refactoring might be a better option. If code is expressive, explaining its purpose becomes unnecessary. Also, comments can become misleading or simply wrong if code is changed but comments aren't updated to match.
    – Pappa
    Commented Sep 8, 2013 at 18:09
34

There seems to be a huge aversion to creating a function in JS. This aversion causes people try to be clever and use ridiculous tricks just to keep stuff in one line like a function call would have been. Of course the function name in a call acts as extra documentation too. We cannot attach a comment to a tricky expression because then it would defeat the point of doing it so we just call it "js idiom" and suddenly it is understandable.

Javascript is extremely accessible, most people don't eat specifications for breakfast like we do. So they will never understand what the hidden assumptions and edge cases of an idiom is.

x = x || 'default_value';

The average joe will either not understand this or has memorized that it is the idiom for default value. Both are harmful, in fact the latter is even more harmful. He will not understand the assumptions and edge cases here. He will not care to read the specification and understand it ever.

When I look at that code I see "if it's null or undefined, then set it to this default value. Although it will also implicitly treat +0, -0, NaN, false, and "" as not suitable values. I will have to remember that 3 months from now when that needs to change. I will probably forget it.".

The implicit assumption is extremely likely to cause a bug in the future and when your codebase is full of tricks like this then there is no chance you are keeping them all in your head whenever you are thinking about what a modification will affect. And this is for the "JS pro", the average joe would have written the bug even if the requirements were to accept a falsy value to begin with.

Your new snippet has more familiar syntax but still has the above problem.

You can go with:

function f(x) {
    x = valueOrDefault(x, "default_value");
}

Now you can have very complex logic to handle the edge cases and the client code still looks beautiful and readable.


Now, how do you differentiate between advanced language feature like passing a function as argument or a clever trick like || "default"?

Clever tricks are always operating under some hidden assumptions that could be ignored when the code was initially created. I will never have to modify an IIFE to something else because a requirement changed, it will always be there. Maybe in 2020 when I can use actual modules but yeah.

| 0 or the cargo cult version ~~num used for flooring assumes positive and 32-bit signed integer bounds.

|| "default" assumes all falsy values are same as not passing an argument at all.

And so on.

5
  • 4
    You're concentrating on one example. What about using stuff like IIFEs, closures, function references? That's the main point of my question. Commented Jul 3, 2013 at 13:23
  • 1
    @FlorianMargaine You don't think I addressed that in the second part well enough?
    – Esailija
    Commented Jul 3, 2013 at 13:24
  • 3
    Well, it doesn't say anything about how to handle the situation where I simply use "advanced language feature", that teammates misunderstand as "clever trick". Commented Jul 3, 2013 at 13:26
  • I like this answer +1, I think it misses a big part of the question, but it talks about other parts and scenarios about it in depth and explains the problems in other team developers picking up such concepts on their own without guidance from reading your code. Commented Jul 3, 2013 at 13:27
  • @FlorianMargaine you mean how to actually handle a situation in practice at your workplace where you are using IIFE and someone thinks that is a clever trick? Like I explained, since there is no hidden assumptions, a memorization like "variables will not be global" will work just fine for the average.
    – Esailija
    Commented Jul 3, 2013 at 13:31
23

You should not lower your programming skill, but you might need to adjust how you write code. The goal, almost above all, is to make your code clear to the people who must read and maintain it.

Unfortunately it can be a bit of a judgement call as to whether any particular style is "clever" or just advanced usage. The code in the question is a good example of this - your solution isn't necessarily better than the other. Some will argue it is, some will disagree. Since both solutions have effectively equal runtime performance (read: the user will never know the difference), pick the style that the team as a whole is most comfortable with.

In some instances you need to teach them better ways to code, but at other times you need to compromise in the interest of clarity.

2
  • +1. Neither specific example the OP gave is empirically better than the other, they're merely different. Commented Sep 7, 2013 at 13:19
  • very nice answer @bryan-oakley. Cheers
    – Andy K
    Commented Sep 14, 2017 at 9:54
7

This may have already been said in another answer, but I would like to answer this question my own orders.

General Guideline

When you work on a team, you are not the target audience of a piece of code. Your audience is the developers of your team. Don't write code they can't understand without good reason.

  1. Unless there is a specific downside to it, all of the code should be written following a specific pattern or guideline that will allow easy maintenance by the developers who will be maintaining it. (A caveat: Following bad patterns just because they are currently in the code base is terrible practice.)
  2. If you can find a good reason to use a language specific idiom that isn't easily readable by the target audience, add a comment. If you find that you need to add a comment to every other line, you may want to rewrite your code to be more readable by your audience. I don't find it valuable to be idiomatic for the sake of being idiomatic.

Specific Example

We have a large number perl scripts in our code base. We typically only use perl for very simple operations and the vast majority of the code is written by java developers, so it's styled much like java. We have a set of perl scripts and a framework that was written by a 'perl guru' that has since left our firm. This code contains many of the more obscure perl idioms and none of our developers, including myself, can read this perl code without extended effort. We often curse him for it. :)

0
5

If you write good code but you think your present or future colleagues may have difficulty following it, you should add a brief comment to explain it.

That way, you might teach them something without insulting their individual intelligence or embarrassing anyone in a group discussion.

3

I would not call your example a trick, but just idiomatic. If you should use it depends IMHO not so much on the current level of your team, but if (at least some of) your team mates are willing to learn some new idioms. Of course, you should discuss this topic with them and not enforce this style on them. And you should not ask them to learn each day 5 new things or "tricks". But honestly, if you have only teammates are not willing to learn something new, even it is so simple and small than this idiom, you should consider to change to a different team.

3

Reading this question and the subsequent answers and discussions there seems to be two points. The first: is it OK to use advanced language features? The second: how can I do this without appearing as though I'm 'showing off'?

In the first case, it makes sense to use improvements and advanced features. For example: in C# you don't have to use Linq or Lambda expressions but most people do because it makes the code tidier and easier to understand, once you actually know what it's doing. At first it just looks strange.

People get used to patterns and in many cases people use the set way of doing things just to get the job done. I'm as guilty of this as the next man. We all have deadlines. In some respects you are guilty of introducing new ideas and new ways of thinking! This comes on to the second point and this is probably where you might be encountering the most resistance.

To the person using the website they don't care which style is used all they care about is does it work? Is it quick? So, if there is no performance advantage to your way then there is no right way or wrong way in the example you give. Does your way make code more readable or not? It might do once your colleagues are used to it.

So, how do you introduce these changes? Try and have discussions with your colleagues along these lines : did you know that this function can be written this way? Code reviews and pair programming can be good times to allow for 'cross-pollination' of ideas. It's tricky for me to prescribe what to do because I don't know the environment you are working in. I find that some programmers can be very defensive and resistant to change. Again I've been guilty of this. The best way to work with these kinds of programmers is to spend some time learning what makes them tick, learn their background and then compare and contrast your styles and experiences with theirs. It takes time but it is time well spent. If possible try and encourage them.

2
  • If you think it'd be easier for you to examplify what you mean in a C# environment I doubt OP wouldn't mind - I sure wouldn't. This question isn't about JavaScript :) Imagine giving up optional parameters, or lambdas in your code because other team developers do not understand it - would you do that? I think you raise some interesting ideas here but if you stop worrying about the specific language you can write it in a more compelling way :) Commented Jul 10, 2013 at 20:22
  • 1
    I work primarily with C# so that was the example that sprung most readily to mind. You do make an excellent point, with regards to whether I would give up useful language features just because others aren't aware of them. The answer would have to be no, but of course the tricky bit is getting others to see the advantages of this new way, which seems to be Florian's main problem. Commented Jul 11, 2013 at 8:55
3

Just don't go work for the Royal McBee Computer Corp then, because who's to say you're not the inexperienced programmer.

sure, its great to write code that's terse and short and it might be useful in a javascript environment (well, until someone produces a js compiler to download to browsers, but that's another story).

what is important though, is the ability for your code to live past the few minutes it took you to write it. Sure, its quick and easy and you can chunk it out and move on, but if you have to come back to it years later, that's when you might think "which muppet wrote this", and realise it was you! (I've done that, sure most people have too.. I blame the overly aggressive deadlines, honest).

This is the only important thing to bear in mind, so while I'd say yes - go with that particular operator if it works and is clear, and your 'inexperienced' devs (though, that is being derogatory to them, I know plenty of inexperienced devs who know all the operators and tricks as they've memorised various web page tutorials and references, they write the worst code even though they know every little trick... there might be more to this than coincidence)

Anyway, if you could read the story of Mel, you'd realise the tricks are not the best thing to put in any code, even though Mel was a real programmer of the first order. This puts paid to any argument where someone says they can write good code and everyone else needs to learn more to keep up.

1
  • 1
    I don't know a single programmer who hasn't went back to their code (from a month ago!) and went "who the heck wrote this". We always evolve in style (at least we try). In this specific case OP is writing standards code, not WTFish code. OP is not discussing writing "clever" code or "shorter to be cool" code, it's idiomatic JS. Commented Sep 11, 2013 at 10:07
2

Well, for starters that looks like basic JS to me.

But in general -- you should not be using clever hacks, to paraphrase "debugging is twice as hard as programming. If you write code as clever as you can, then you are by definition unable to debug it".

That does not mean that you should avoid code just because others wouldn't understand it -- you should write the code in as clear and consistent manner as you can. But your criteria for clear should be "will I understand this on first reading in a year", not "can anyone understand it".

Write in a clear manner, that you have no difficulty understanding and let others work on increasing their skills -- don't handicap yourself in order to save others some hypothetical trouble.

1

I'd discuss with my teammates what kind of coding standards we want to have as this is mostly about how should something that can be done in dozens of ways be done for our code base. If there is a consensus that would be my initial attempt at an answer.

If there isn't, then I'd likely consider what kind of proposed standard makes sense and start putting that into practice once I've cleared it with management and some of the team. The idea here is to make sure that management is OK with this idea and that I'm not just going off doing my own thing and then forcing everyone else to take it.

I'd look at this more like the question of what kind of standards and practices does your team have rather than just skill level as there are many ways to evaluate code. How well can others maintain it is one of those criteria.

1

The problem is that you want good readability of the source, but readability is in the eyes of the beholder.

I would suggest that we need better tools to solve this problem. Nothing complex, mind you, we have had the technology to do it for more than 50 years. Include a parser in the editor, and have the editor save the source in form of sexps (yes, just like lisp). Then the source is read, the editor unparses it into the syntactical and typographical (you know, spaces, tabs, commas), form the user prefers.

This way, you can type and read x = x || 10 and other programmers will read it as

if (0 == x) { x = 10;}

emacs has all the pieces to do that easily.

1
  • 1
    In this case we know who the beholder's are. They are our co-workers. I think that phrase is normally used when you don't know your audience.
    – dcaswell
    Commented Sep 8, 2013 at 0:18
-1

Rather than dumb down the code, why not improve the quality of the team? Training, coaching, education, and improved hiring practices can do a lot towards ensuring continuous improvement.
Statism, code rot, refusing to improve and innovate because someone doesn't want to work on self-improvement only cause trouble down the line, and sooner rather than later.

Of course in the specific case you show, you're just trying to be clever and deliberately writing obfuscated code, which is never a good idea. Code should first and foremost be readable, readily understandable, not written to show how clever you are at creating something in the least statements possible (special cases excepted, like where more statements would lead to unacceptably poor performance, in which case copious comments are called for).

2
  • 5
    In this case, I'm not being clever. I'm writing idiomatic code for any experienced dev. Do you see why I'm struggling now? :) Commented Jul 3, 2013 at 12:05
  • 3
    Your first paragraph is spot on, but -1 because your second paragraph is way off the mark. It's inaccurate to say this example is a deliberate attempt at being clever. It's actually very clear, and more importantly, it is an idiomatic style that many good javascript developers agree on. It's not the only idiom in javascript for default function parameters, but it is a common one.
    – Ben Lee
    Commented Jul 8, 2013 at 23:38

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