
Ask HN: How do I get my team to write better code? - kamakazizuru
I&#x27;m leading a team of programmers - not all of whom I had a say in choosing. They&#x27;re generally a smart lot when it comes to understanding the algorithmic &amp; functional logic - but make beginner level mistakes like hardcoding thigns, naming functions like &quot;findeventsfromthelast6hours&quot;  (when the 6 hours could be changed at some point - rendering the name of the function pointless). And so on...<p>I review commits now and then and find some of these issues - but due to the nature and timeline fo the project cannot do it for each and ever commit of course. So I&#x27;m wondering what  I can do about this? Is there a short guide or book that I can recommend my team to read? I of course tell them these things myself too - but it gets frustrating because I keep feeling like a lot of this should really just be common sense.
======
rmcastil
You need to stop thinking of them as 'coders'. When you think of them that
way, you're putting them in a box and limiting them.

Instead consider them to be a team of people. As you said, each of them are at
different stages in their career in programming. You need to understand this
and sympathize with them and their code. This takes time and effort on your
part to help lead the way.

You say that you review the commits now and then and find some of these
issues, but do you bring them up? Do you show them how the code can look and
read better?

If you do, make this a process and act as the person approving pull requests
for the first several weeks. Then formalize this process and delegate this job
to the next person who you feel writes high quality code.

Again a SHORT guide or book will not turn around the quality of your codebase
and the team working on it. If you're interested in better code, invest in
your team. Send them to conferences, give them a budget to spend on books,
have book clubs and lunch and learns. Invest in the process of improving the
skills of your team.

~~~
BillyMaize
I agree with this other than the lunch and learns. Leave their one break in
the day out of it.

~~~
StevePerkins
Heh... have a bad experience with lunch-n-learns at a previous gig? :)

Pretty much everything in this comment thread describes approaches that only
work with the buy-in and support of upper management. If OP (or anyone else)
is truly in a position where code reviews are impossible, and lunch is
everyone's "one break in the day", then there is very little you can do
improve code quality in that environment.

In THAT kind of environment... you slog through, get what you can onto your
resume in preparation for the next job search, and don't let yourself get too
emotionally attached. Support for quality has to start from the top, you
really can't "grassroots" it if upper management pays only lip service or
doesn't really care at all.

------
npsimons
There are many different things you can do, but it all comes down to: do your
programmers care? If they don't at all, it will be impossible to make them
improve. If they care a lot, they will improve themselves. If they care a
little, that's where you can try some of the strategies others have given
here.

As for my specific suggestions, I would recommend "The Pragmatic Programmer:
From Journeyman to Master", and actually apply the principles in the book
yourself. Use unit tests, setup CI (such as Jenkins), a code review system
(such as Gerrit), a bug tracker, code analysis (such as Sonar), and use tools
(linters, etc). Also, cranking up warnings on your compilers/interpreters and
not allowing in anything that doesn't pass muster is a good start. This might
all seem like a lot, but you can do little parts of it at a time. It is
worrisome that you say you can't review every commit. Perhaps you could assign
members of your team to review each other's commits? This would also have the
bonus of giving them a feel for what it's like to be on the receiving end of
bad code, and to teach each other.

------
edw519
_I review commits now and then and find some of these issues - but due to the
nature and timeline fo the project cannot do it for each and ever commit of
course._

Why not? I do.

I have 8 direct reports and I review every single line of code that they
write. Some days this is all I do.

Others think I'm crazy but our group outperforms every other group by any
measure. I'm convinced it's because of peer review.

A few thoughts:

1\. Regarding resources and timelines: It's not when you start, it's when you
finish. Every hour of peer review saves me at least 10 hours on the back end
(systems testing, regression testing, user acceptance testing) and 100 hours
on the back/back end (maintenance).

2\. I never have to tell anyone anything more than once. I make sure I explain
why. (When you do <x>, you cause <y>.) Then they learn and remember. This is
not a religious debate; it's a way of conducting business. This won't work
nearly as well if people are more interested in showing what they know than
learning how to do it properly.

3\. As I get to know different people, I know where I can skim and where I
have to focus. I rarely return much to senior people. I often return a lot to
junior people.

4\. Everyone makes mistakes, even senior people, and even me. Peer review,
like regression testing, is a pain in the ass, but it more than pays for
itself on the back end.

5\. As soon as you feel you're ready, you're senior people can take some of
the peer review workload. I have done this before, but it hasn't lasted
because of turnover.

6\. Once people know everything will be peer reviewed, they become more
careful about what they're doing.

7\. Technical debt is a huge timebomb. I know of no better way of slowing it
down than 100% peer review.

8\. Everything is documented in the ticket. This provides some feedback on how
it's working and what should be changed (programming standards, peer review
methods, etc.)

9\. Once you put your phone away, get off the internet, and stop going to
useless meetings, it's amazing how much more time you have to do things that
really make a difference, like peer review.

10\. I hate doing this. I'd rather just code. But the only thing I hate more
than peer review is fixing bugs. That's why I do this.

~~~
circlefavshape
+1

100% peer review is the _only_ process change that has been implemented
anyplace I've worked in the last 18 years that seriously and lastingly
improved code quality. I don't like reviewing, but it's very _very_ effective,
especially once you get good at it (and, like anything, it takes practice) -
also, fwiw, it really improves you own coding

~~~
HeyLaughingBoy
I agree completely and I would add that you need to keep the review size
reasonable for this to work. I've been asked to review thousands of lines of
code at once and it doesn't take long before your eyes glaze over and you
start missing things.

------
jgrahamc
I wrote a short blog post for work about this
[http://blog.cloudflare.com/making-code-better-with-
reviews/](http://blog.cloudflare.com/making-code-better-with-reviews/)

Simply put: we use git and developers can't commit to the master branch. They
commit to their own branch, the build system builds/tests that branch and the
developer makes a pull request and gets the code reviewed.

~~~
kevincennis
I enthusiastically second the suggestion to use pull requests. They make a
huge difference in my experience, for a few reasons:

1) You're more likely to catch bugs, security issues, and potential
performance problems before they get into your main branch.

2) Your developers will get feedback on their code, which is hugely important
to growth (you can't learn from your mistakes if you don't know you're making
them).

3) Your developers will learn how to _give_ feedback, and how to have
constructive discussions about code by virtue of reviewing and commenting on
pull requests.

4) I think there's some light, healthy social pressure when you know that
other people are going to be doing code reviews. People seem to be a bit less
likely to take the "easy" (read "hacky") way out of a problem when they know
there will be eyes on their code.

My experience has been that most developers really _like_ getting feedback and
having these discussions. And I've definitely seen them pay off.

------
andreasklinger
We do team based pull request reviews. (in github)

You need two other seniors to give you +1 to move on. The number "two" comes
from the fact that we are still a small team.

People can will still review stuff that has already two +1's but the commiter
could already move on if needed.

The advantage of pull requests is less "teaching how to do it properly" but
more "learning from each other" and "continous exposure to all parts of the
codebase".

Thus it's not the job of the Team Lead to do this reviews but everyone's.

Pull requests reviews are done (asynchronously) done each morning (whatever
this means for each person/timezone). This leads to the fact that you usually
can expect PRs to be done within 12-36hours.

I could not imagine working in a team with different experience levels (not
speaking of countries/timezones/specialisation) without pull request reviews.

~~~
cscharenberg
This is a good process and works well for my company as well. Everyone does
code reviews but you need 2 senior people to call it complete and check it in.

The other part is encouraging them to be engineers, not "coders". Coding
implies write code and shoot it out. Engineering is a mindset of continuous
improvement and complete examination of the problem and solution.

------
blueatlas
We do 1-1 peer reviews and make changes in real-time so that the developer can
see the preferences for code style, naming, indenting, etc.

You should also lead by example. Your post includes numerous spelling and
grammatical errors. If you want them to be precise with such things, you
yourself must do the same. Sorry, not picking on you, but your developers will
follow your lead with any form of writing.

~~~
kamakazizuru
Right, well it's one thing when posting on HN - I'm not really running a
spelling and grammar check. That said - my complaints are not around their
writing style. Many of them are non-native speakers and I can live with bad
grammar & spelling in comments. The pain-point is really the coding style that
leads to bugs where they could have easily been avoided.

I do get your general point of leading by example though - and like the idea
of "show them whats wrong instead of just telling them by fixing it in real
time".

------
mamcx
Apart of the suggestion here, you have a problem in " but due to the nature
and timeline for the project cannot do it for each and ever commit of course".

If the time is so tight that you can dedicate time to fix stuff (including the
code quality) then any of the suggestion here are DOA.

Remember: If you want to chop wood for 10 hours, use 8 to sharp the axe.

Code, code, code, code non-stop is as efficient as chop wood without sharp it
after a while.

Dedicate time to improve the code quality _ALWAYS_ pay off. Think: "I'm time
constrained, so continue the death march until we finish" ALWAYS make the
project _take longer_.

\---- A good idea is STOP each week, review the tasks, ask where are the pain,
where is the code that is becoming a mess, what is complicated, then FIX that.
Decreasing code/clearing it in the short AND long run will be a time saver.

Encourage change when the purpose is quality. Not stick to code/design that is
a pain. Stop each week to breathe and see if move forward as now make sense,
or is better to kill a portion of the code and rework it.

This is specially important when the TIME is critical. When time is not
critical, you can do bad code because, hell, you can take the time! But when
you have not time, any bad code is slowing you.

Then make this clear to the team.

Not be yourself the only that code review. Everyone can do it.

Take control when deploy. Not when your customer say so. Be predictable (ie:
Always deploy in the start of the week, never at the end).

------
carlio
I think you can get a long way with "automated code reviews" \- basically,
linting and static analysis. There are various ones out there for your
language - disclaimer: I wrote one for Python called
[https://landscape.io](https://landscape.io)

They can take a little while to get set up exactly how you like them, but once
you do, run them on your CI server after every commit and it'll output the
warnings about style violations and so on. That's basically what you're after.
It's not a replacement for a manual code review but it's certainly closer to
what you want than nothing!

As other have said though, this only works if you and your team treat code
quality as important and actually take the time to understand the output of
the analysis tools. That's why I like the CI approach, so that you can see if
the number of warnings is going up or down over time and react accordingly.
That metric is a useful one to keep an eye on to get a view on the quality of
your code, and if it gets too high you can schedule some time.

(PS if you are interested in learning more about "Automatic Code Reviews" for
Python, here's a talk I did at EuroPython14:
[http://carlcrowder.com/pages/europython-2014-automatic-
code-...](http://carlcrowder.com/pages/europython-2014-automatic-code-
reviews.html) )

~~~
kamakazizuru
this is great - I love coding in Python and would definitely use this in the
future. Currently we're doing everything in Node.JS though - do you know of
any tools for that ? We do use Jslint etc - but landscape certainly seems to
go a step further..

~~~
carlio
You should check out [https://codeclimate.com/](https://codeclimate.com/)
then, they support JS and Ruby. There's also [https://scrutinizer-
ci.com/](https://scrutinizer-ci.com/) who do PHP and apparently Python and
Ruby now too.

~~~
paulblei
[https://www.codacy.com/](https://www.codacy.com/) supports JS, PHP, Scala,
Python and CSS

------
jasallen
Honestly, the things you describe are not that bad. If they are writing code
that passes the test cases, move on and be happy. If you'd described things
like circular dependencies and passing brittle "magic strings" through
multiple layers -- things that will GENUINELY cause maintainability issues I'd
sympathize more.

I doubt anyone named it the "...last6hours" but then parameterized the number
of hours -- so you're changing that code anyway if the number of hours
changes. If you have a requirement to parameterize that, do so, otherwise
YAGNI, move on.

Hardcoded strings is another one. Frankly, unless you are using the string in
more than two places or it's part of a "set" of values, where referencing the
full set is useful, you may just be making more work to do it 'correctly'.

You reference timeline pressure. "Good, Fast, Cheap, pick two". If you don't
have enough programmers for the timeline alotted, the powers-that-be have
already selected Cheap and Fast. Deliver a product that does the job asked, a
few maintenance iterations (if the product is one of the 30% that make it that
far) will teach you want areas of code are touched a lot and need to be "made
maintainable"

~~~
kamakazizuru
Thanks - while a lot of the other advice on this thread is helpful in terms of
being a better team lead in general - I think the stuff you mentioned is
really what I can apply almost immediately - especially the last bit about
makign things maintainable.

------
shreeshga
Give them a free copy of Clean Code[1], I started to care about my code more
after I read that book.

You cannot sit and review each commit, the best you can do is to make them
care about code.

[1] [http://www.amazon.com/Clean-Code-Handbook-Software-
Craftsman...](http://www.amazon.com/Clean-Code-Handbook-Software-
Craftsmanship/dp/0132350882)

~~~
avelis
Any chance I get I make an effort to suggest this book. There is also another
book by Robert C. Martin which I suggest called The Clean Coder[2].

[2][http://amzn.com/0137081073](http://amzn.com/0137081073)

------
DanielBMarkham
Saw something interesting at a conference a while back. If you have a few
days, it might be worth trying. It's called "mob programming"

Basically you get everyone in a room. The basic premise here is that everybody
brings their "A game" and shares it with everybody else. One person types,
everybody else decides what to type. Sounds chaotic, but as long as folks stay
focused, what happens is that very quickly the entire group gets on the same
page about IDE, coding standards, patterns, and whatnot.

I'd plan for a few days, but you might see results just in a solid morning of
doing it. Don't know.

Of course, you probably don't have a few days. But -- how much time are you
losing to this other stuff? Couldn't hurt. Beats giving people a book to read.

~~~
brianmcc
I find this both crazy and quite appealing! Maybe doing so after also dishing
out some of the good books recommended elsewhere in the thread could work
well. First read about good practice, then participate in it?

~~~
DanielBMarkham
Yes. It struck me as one of those "crazy enough it might work" things.

Looking forward to trying it!

------
jonkaye68
My advice is that before you propose solutions, you ought to really figure out
what the problem is. Is it ignorance, laziness, or something else? I've looked
over the various responses and haven't seen one angle -- have you asked your
team about why they are not doing what you'd like? There's a lot of advice
about read this, do that, etc. but you don't yet know why they are not
following what you think is common sense. You might find something surprising
by putting it on the table, or at least you'll find the reasons they give.
Then you can address those. My advice is to put this out as a question to your
team and see how they react. You might even find out that some of the patterns
you think are best are not, i.e., your team has a reason you didn't think of.
For example, I created a 10-element array to speed the search of a big table
in which all keys began with a digit from 0-9. The more experienced guy told
me that in the .NET world, the convention was to use key-value pairs instead
of an array. To satisfy him, I changed it to a hash table, where I hashed the
first digit. It seemed pretty silly to me, making a hash table for what to me
was clearly a better approach using a simple indexed array. He had in his mind
the "right" way to do it, eventhough I still think the hashtable in this case
was silly.

------
Bahamut
If your timelines are such that you cannot institute a process for code
reviewing pull requests and you have a team of developers, then I have to
question the planning by the managers.

------
vladimirralev
I honestly think this complaint is poorly argumented. It depends on the nature
of the project. When you say "findeventsfromthelast6hours" that's perfectly
reasonable name for disposable code or private implementation. You should only
be concerned about such name if it made it to the API somewhere. Hardcoding
things is also not a bad thing by itself. If I had to guess, your team is
under the impression the code they write is prototype and will be rewritten
anyway, not API to be consumed by other parties or doesn't have a direction
that allows them to make the proper abstractions. Sure you can demand nothing
to be hardcoded, but you must remember eventually those params will have to
come from somewhere. I see so many people move hardcoded values Java -> IOC/DB
config or Erlang -> DB not realizing it's more difficult to reinitialize the
component than to do a hot reload and speed up your development process at the
very least. Whatever you do it must be for a well-defined reason. It's also
possible your team developed their own process where it's easier or faster to
fix these issues on the go rather than planning in advance.

~~~
cellularmitosis
Indeed, I think Robert Martin's "Clean Code" would agree that if you don't
need to vary the "6 hours" part at run-time, then
"findeventsfromthelast6hours" is actually better than passing in an "hours"
argument ("Clean Code" advocates minimizing function arguments). Refactoring
the name in the face of a one-time change in behavior (e.g. "4hours") should
be trivial with any modern IDE.

------
llamataboot
Clean Code and The Pragmatic Programmer are both great books, but really your
team needs to be doing code review on every line of code that is going to make
it to production. It doesn't have to be /you/ reviewing all of it. As a bonus,
having your team review each other's code builds more familiarity with the
app(s) you are working on and allows all your team to learn from each other.

------
joezydeco
Steve McConnell's _Code Complete_ might be a good resource. Could you buy a
copy for each team member and take an hour each week to read and discuss a
chapter? That might get you on the road to better coding practices.

[http://www.amazon.com/Code-Complete-Practical-Handbook-
Const...](http://www.amazon.com/Code-Complete-Practical-Handbook-
Construction/dp/0735619670)

------
rglullis
Each week, you take one of them to be your pair. Do pair programming for 2-3
hours each day of the week. Make a wiki page with code style guide and
recommendations. Each common mistake you find your pair doing, add to the wiki
page. At the end of the week, you and your pair review the commits you have
done _and_ from a couple of others from the team.

------
tamersalama
They are not coders. They are partners in success. Make sure first you treat
them as such. Each person might have much to give above turning specs to code
(ideas, processes, improvements, automations, design, architecture, etc...).
If they feel they have a stake and their efforts are accommodated (and
praised), there will naturally be an improvement.

~~~
thegenius
Yes. This. The OP is NOT leading a team of programmers. He is asking HN to
lead his team of programmers. "lead" is not a noun. It is a verb!

------
j45
Sometimes it's more about learning clever architecture than clever, or better
code. Clever architecture can often help keep your code an order of magnitude
simpler.

The issue you may run into is seasoned developers are the ones who learn this
over time.

Conversations based around "why" help a team understand the importance of:

\- creating a codebase that they will still want to / be able to work on
without dealing with poor decisions of the past, o

\- being kind to your future self

\- realizing not everyone has had a relationship with a codebase for 3, 5, or
even 10 years and what that might look like. In the real world, existing code
bases are a reality. Optimizing for that reality, instead of the clean slate
that university assignments, or startups provide can also be an important
light bulb.

\- peer reviews, and teaching the why you do things a certain way are far more
likely to get the lasting result you are seeking.

------
smanatstpete
Try having your team read these books:

[http://www.amazon.com/Writing-Solid-Code-Microsoft-
Programmi...](http://www.amazon.com/Writing-Solid-Code-Microsoft-
Programming/dp/1556155514)

[http://www.amazon.com/Practice-Programming-Addison-Wesley-
Pr...](http://www.amazon.com/Practice-Programming-Addison-Wesley-Professional-
Computing/dp/020161586X)

[http://www.amazon.com/Code-Complete-Practical-Handbook-
Const...](http://www.amazon.com/Code-Complete-Practical-Handbook-
Construction/dp/0735619670)

I know these are old books and are C and C++ oriented, but it helped me a lot
during my formative years and helped me transition from being a decent
programmer to being a decent engineer. They are short books which are well
written and not very dense.

~~~
edgo
Code Complete has 960 pages, not in the short book category

------
canterburry
To me, this is the difference between being a programmer and a software
engineer. Software engineers think of the bigger picture and understand the
overall impact of choices made in the code on other things. Programmers focus
too much on just coding and don't see the bigger picture.

I am sure much of this has to do with experience and having had the
opportunity to learn from good software engineering teams.

In my teams, I have handled this with mentorship. Recognize who in the team
needs mentoring and assign them a code review buddy. If mentorship somehow
doesn't work, you just have to manage those programmers and put them under
tighter supervision. However, you can't expect much change until you set and
communicate expectations. Be clear on what you expect and review against those
benchmarks.

------
StevePerkins
What I've seen work is a combination of code reviews and ongoing continuing
education.

\----- CODE REVIEW SIDE -----

This can include automated tools, human interaction, and computer-assisted
human interaction. To start with, pick a set of command code standards and
style guidelines. Google has nice ones for many languages that you can use as
a starting point ([https://google-
styleguide.googlecode.com/svn/trunk/](https://google-
styleguide.googlecode.com/svn/trunk/)).

Secondly, enforce them through automated tools (e.g. in a Java shop, these
might include PMD, FindBugs, Checkstyle, etc). Have everyone install the IDE
plugins for those tools (along with your common config). Incorporate them into
your continuous integration build process. Perhaps put in place a static
analysis tool like Sonar to send regular reports on code issues.

The human element would start by having someone review committed code before
it's allowed to be merged downstream. Git's pull requests, along with diff
view systems like GitHub's, make this easy... but you can use workable
practices with Subversion or whatever also.

There are limits to "pull request"-based code reviews, though. When change
sets grow too large, it's difficult for the reviewer to understand the high-
level vision for the changes. You start reviewing the trees, rather than the
forest. So I think you still need periodic in-person code reviews, to walk
through the state of a codebase at certain milestones and discuss higher-level
issues.

\----- CONTINUING EDUCATION SIDE -----

The thing that I've seen work best here is a regular, ongoing lunch-and-learn
type series. Take any of the standard code quality treatises (e.g. "Pragmatic
Programmer", "Code Complete", "Clean Code", etc), and tackle a chapter per
week. Better yet, split up the chapters and have the team members themselves
present a chapter. It's great experience for them (if they're not TOO freaked
out by public speaking), and you'll never learn something as deeply as you do
through the process of teaching it to others.

The problem with lunch-n-learns is that you MUST have buy-in from management
to make them a serious and ongoing part of your team culture. When deadlines
are tight and there are fires to put out (and when are there not?), a lot of
companies are quick to cancel that week's lunch-n-learn. When that happens,
they lose traction and fall apart.

------
euphemize
Most tips here are very good, suggesting books ("Clean Code", etc.), team
huddles and showing them the way. But you seem to be implying that they could
be great coders - if they wanted to.

I can't help but feel like these programmers aren't very passionate about
their code, and that's what you need to help them with. If they're naming
variables wrong and hardcoding strings, but understand the asymptotic running
times of what they're doing, I'd go on a hunch and say they're bored or
unmotivated.

I say this because I recognize myself in some of these behaviors - when I want
to get rid of something and don't care much about the project. "What's the
fastest shortcut I could take to get me out of here?".

------
thothamon
Lots of good advice, but I suggest having the whole team review each others'
code 100% of the time. In other words, each commit gets a random or rotating
selection of two people who review the code.

Reviewing other people's code is almost as valuable as being reviewed. Even if
it's not perfect all the time, it builds a good culture of coding and gives
your developers valuable experience.

The culture of the code review should be a careful examination with Q&A. You
need to take as long as you need to get it right. This is a great investment
in your product and your team that will pay off starting right away and
extending into the long term.

------
codingdave
It is your team, with your own culture, so I would not presume to tell you
exactly how to do it, but I can give one general bit of advice:

Communicate.

Tell them what you are seeing, and just as importantly, why it can be a
problem. Many times, beginner mistakes arise simply because they never have
been exposed to the reasons for them being "mistakes" in the first place. So
teach them. Nicely.

Think of them as students, and you are their teacher/mentor. Help them grow,
support them and act as a resource so they can do their own jobs better. Treat
the whole experience as an opportunity for growth, not a judgment on their
skills.

------
jtwebman
Below are a few things that could help but you said it yourself. If you don't
have time to review then my guess is you are also pressuring the devs for
time. Sometimes to make your deadlines they might have to cut corners. Maybe
you can switch to a more agile process and instead of crazy deadlines you ask
them what they can get done the next 2 weeks (correctly).

\- Setup process for them to review each others commits (Code Reviews)

\- Add a tool like FxCop, JsLint or JsHint to the build and commit process

\- Hire more senior programmers that they will look up too.

\- Paired programming

\- Be a team and do stuff as the team (We are not factory workers)

------
eccp
Point them to some coding guidelines for the language/platform which you're
developing.

For Clojure development: [https://github.com/bbatsov/clojure-style-
guide](https://github.com/bbatsov/clojure-style-guide)

For Scala: [https://github.com/alexandru/scala-best-
practices/](https://github.com/alexandru/scala-best-practices/) or
[http://docs.scala-lang.org/style/](http://docs.scala-lang.org/style/)

------
arvanhalleorg
Technical books. Code videos from stuff like Google I/O and friends. Coding
conventions. Peer programming. Use pull requests and assign them to another
member of the team. It won't fix it overnight, but you'll notice the changes
as they come in. Also, you might want to let them learn to write better code.
People adapt best when you lead them, instead of getting them to do it.
Everyone was a beginner and not everyone was exposed to the same amount of
code or scenarios.

------
projectileboy
I spent 10 years "coaching" (that word... blech...) teams, and from my
experience there's no easy answers. What works for one team at one point in
time might be horrible for other groups. Having said that, here are some
things that have worked for me:

* If some of the people on the team have more skills than others, you could try asking people to pair program for part of the day. You could try setting up a lab space for this if people are interested.

* Weekly brown bag sessions where you talk about a technical topic of interest. Let people on the team nominate ideas. Do it over lunch and have the company have food delivered. The better the food, the more people will want to attend.

* You should get an automated build in place, and encourage people to do some level of automated testing if they aren't already. Some people take this a step further and use the automated tests to help drive out the design (Test-Driven Development by Kent Beck is a good book on this topic).

* You could try starting a technical book club where people read a chapter a week, and then get together and discuss it over lunch. Given your situation, I'd strongly recommend starting with "The Practice of Programming" by Kernighan and Pike. Again, it helps a lot to have the company get good food delivered.

Don't make a million changes at once. I might start with the book club if I
were in your position. None of this stuff will magically transform your team,
but these kinds of things can start creating a culture where people start to
care more about the quality of their work. You'll encounter people who have
very strong religious feelings for or against any one of the things I listed
above, usually based on some past experience. I've seen each of the above
succeed big and fail hard with different groups. Just try what seems to make
the most sense for your team.

One last note: when you try things, don't do stuff in fits and starts - talk
with the team about what you're going to do, see if there's buy in, and if
there is, commit to trying it for, say, four weeks. After the four week period
is done, talk with the team about whether or not it's working for them, and if
they'd like to stop or continue. Over time, you can add/remove/change more
things in the same manner.

Good luck!

------
nXqd
Short term solution : Lead by example, a good chance that they can learn from
those and maybe copy them afterward. But the code quality will improve. This
can take your time but as long as you have several good examples, you can
reuse those. Long term solution: Encourage them to watch conference and good
books like Clean Code, or TDD books. Those are incredibly good. Maybe they are
not interested in at first, and never, but those who want to be better will
become ones.

------
lazyjones
You absolutely need coding standards (the stricter, the better, as long as
they make sense), especially with languages like C or Perl where you can shoot
yourself in the foot with what looks like particularly "smart" coding.

I'd also recommend choosing a strict language with useful conventions like Go
(www.golang.org) if you have the possibility to switch. If will prevent many
(not all) headaches in the long run, even if it's not immediately obvious.

------
ainiriand
If you can spend some money I recommend hiring someone good to give some
lessons, some hours per week. Then there is also the option to give those
lessons yourself.

~~~
iandanforth
This. The general form of this question is, "How do I help someone increase
their skill level?"

If the topic were math, you'd immediately see that a tutor and a lot of
practice were essential parts of the equation.

If the topic were baseball a coach and a lot of practice would leap instantly
to mind.

However, it is too often the case that employers and managers draw a line
between the "educational" period of a persons career and the "work" period of
their career. They also draw a line between what is their responsibility (in
terms of improving an employees skill set) and the individuals. Combined these
two lines means it is easy to get into a situation where, as a manager or
employers, you are likely to think "You should have learned this 'before'" or
"You should learn this 'on your own time.'" Neither of which thoughts actually
help get you closer to a more skilled employee.

------
jwitchel
1\. Smaller commits. Break the project down into smaller tickets and have them
check-in more frequently.

2\. Rotate who reviews so everyone gets a chance to review everyone and put
yourself in the rotation.

3\. Lead by example. Write some code into production and submit yourself to
review. Everyone writes code, everyone reviews, everyone gets reviewed.

4\. Each review find just one thing to refactor and let them know they should
be looking for the same thing in other people's code.

------
rovolutionary
You can set up a weekly code review where you take turns letting each
developer present a piece of code they wrote to the team, and then discuss and
give feedback about that code.

Also direct them towards resources where they can learn clean code practices.
Perhaps institute a monthly "book club" where you select books about good
coding practices. I would suggest starting with Clean Code by Robert Martin

------
arisarnado
I think you need to come up with a coding guideline. After that, you
review/discuss it with your team.

Also, you may need to create a "checklist" and encourage your team to follow
it as a way of self-check on their work.

In this way, you will be able to concentrate your reviews on important things
(such as logic, etc) since the trivial errors were already eliminated.

------
feyn
I’ve been in a technical lead position across 5 companies over the last 10
years, and after trying just about everything written or spoken about
influencing team code quality, I finally found a formula that works:

1) Take on the most brutal coding tasks/features yourself. You will have to
cherry-pick the upcoming work to look for things that just about any other
engineer would freak out about if they got it assigned to them.

2) Knock it out of the park in terms of time-to-market __and __code quality.
You need to do both, to eliminate the concern of, “We don’t have time to write
quality code.” The code should be so clean, so well factored, so well unit
tested, that you would be comfortable getting a code review from Martin
Fowler, Kent Beck, and Bob Martin all at the same time. This also implies that
you’re read their books, and would know how to pass their code review.

3) Call a code review with the entire team, and aggressively code review your
own code. There is always room for improvement in any code, depending on hour
you interpreted current requirements and future needs. This has to include
phrases like, “I didn’t know what to do here so I…”, “I really which I had
more time to change this to…”, “I went back-and-forth on how best to abstract
this, but ultimately I chose this abstraction because.” Basically, you’re
looking to show them the worst possible code review on the best possible code.

4) Depending on the personalities involved (including your own), decide how
you are going to coach each engineer 1-on-1. No two people are alike, so you
will need to dial your approach to each person. Some people will thing you are
a show-off, some will think you intimidating, as well any another other human
emotion and concern that causes someone to not seek out or not take advice.
This is where your emotional IQ and communication skills will be tested.

5) When you start coaching them, don’t go after every single flaw. Try to find
the underlying root cause of the problem in the code, and address that. A
common problem I run into is the people just don’t understand OOP but think
they do. I usually have to coach them on basics like encapsulation and
cohesion, working them up to design patterns, and further up to domain
modeling, all the while instructing on proper implementation technique (such
as unit testing.) Once they have the fundamentals in place, you can begin
fine-tuning certain implementation patterns.

6) Over time, at least one member of your team will get good. With their
permission, publicly review their code. In this, you want to be very positive:
“I like what they did here…”, “That’s a good way to encapsulate that problem”,
“That’s a powerful abstraction, especially the way it’s used…” This Rite of
Passage should then become what other engineers are striving for, as only the
best engineers are getting public praise fests.

This basic formula has served me very well with my current team, in that they
are one-upping themselves on code-quality. Still, however, I take on the most
grueling coding tasks to set the example and lead from the front. This acts to
continue to demonstrate my commitment to code quality, as well and lets the
team know I would not ask them to do something that I would not do myself.

------
blahbap
As for books, "Clean code" by Robert Martin is an obvious choice. It covers
the fundamentals of good, clean code.

[http://www.barnesandnoble.com/w/clean-code-robert-c-
martin/1...](http://www.barnesandnoble.com/w/clean-code-robert-c-
martin/1101628669?ean=9780132350884&itm=1&usri=9780132350884)

------
avigesaa
Depending on the language and context, an explicit method name like
"findEventsFromLast6Hours" isn't that bad. If it's just a private helper
method, I'd let it pass.

Empirically, I've found that methods with ambiguous parameters (e.g.
"findEvents(6)") are a much bigger problem (event amongst senior programmers).

~~~
Thetawaves
I also had a problem with this example from OP. It is generally a good idea to
implement the minimum amount of functionality to satisfy the requirements. I
have seen many complex systems fail because it was over-engineered with the
idea that 'we might need this down the road'.

When requirements change, you almost always have to rewrite the code. If you
don't have to rewrite the code, you still have to go back and read it to make
sure it satisfies the requirements. Less code to deal with will make this
process easier. There is no reason to over-engineer today for what might
happen tomorrow.

Code for the here and now, and aggressively delete code whenever possible.

~~~
Thetawaves
If your 'coders' are writing functions like findEventsFromLast6Hours, you are
not giving them clear requirements.

I assume you gave this engineer the requirement 'find all events from the last
6 hours.'

You should have given them the requirement to find the latest events for a
specified interval.

------
yeezul
I highly recommend (to everyone here for that matter) the book Clean Code: A
Handbook of Agile Software Craftsmanship by Robert C. Martin. It deals
specifically with the mistakes you've mentioned in your post, such as
hardcoding and naming functions.

Great easy read. Cheers

------
lukesan
I would start with Refactoring by Martin Fowler. For me this is the
quintessential book when it comes to basic software design principles. It
introduces code smells and tells you step by step how and why to improve your
code design

------
alwendt
If some gross code gets into the tree, nominate it for a code reading with a
team of three or four people. This helps a lot because the code gets improved
and the reviewers' styles also improve.

------
tgreenhaw
Start by watching "The Downfall of Agile Hitler"
[https://www.youtube.com/watch?v=l1wKO3rID9g](https://www.youtube.com/watch?v=l1wKO3rID9g)

------
julie1
Forget all the craps about new silver bullet mantra (peer review, agile and
peer coding and shit).

Just put them on mandatory level 2/3 support 20% of the time for the code that
is written.

When you have to maintain code you fully grasp the concept of «well written
code»: just code that is easy to maintain.

And every month try to have them talk about their top 3 reasons that makes
them loose time when fixing code.

My fear with peer review is that it is very artificial, what I like with
maintaining code is that you see your mistakes sometimes due to trying to
write «clean» code.

Magic number deserves a bullet in the kneecap, but magic numbers disguised as
factories are worse. A good code is a code where mistakes are obvious. Value
these coders.

------
alwendt
Nominate crufty code for code readings. They are great to improve the quality
of the code and that of the reviewers.

------
dharma1
do a half day workshop with your team going through typical mistakes, how to
avoid them, and how to write good quality code - after that there should be no
excuses.

pull requests also essential but it's still admin overhead for you to review
everything

------
devb0x
PRs work. Pull requests serves to manage quality but also can find issues.

------
thegenius
the best way you can improve the quality of code is to LEAD BY EXAMPLE. you're
not alone in that you've been given a leadership position in a circumstance
outside your control, and it's naive to think that even if you were given your
pick of the litter, that they'd all be A players who respect you and don't
want your job, have issues with your quality, etc.

You absolutely cannot control people. The only way to get people to do things,
and i mean the ONLY way is to get them to WANT to do it. Therefore, you have
to ignore the people who aren't cooperating with you and focus on the one's
who do. Treat them like gold and be willing to make compromises with your
quality standards because they will not be the same as yours. Then, as you
make inroads on the project together, the rest of the team members either have
a choice to get on board or get left behind. It is that simple.

------
a3voices
Good coders should start new projects. Mediocre coders should maintain
projects.

------
waps
I work at a place that requires code review and has automated checkers for
"code quality". Let me tell you the obvious : the best code I see has these
problems, because the best code is the best code because it uses the language
to it's maximum effect, which often precludes having perfect "style". The best
code is tested in functional ways, which means that it doesn't have "good
coverage", nor does it really use unit tests more than a little bit.

And the worst code I see, by people who learned to code a month ago or worse
and don't know any algorithms (but feel like they know better than people
who've studied algorithms and languages for years) almost without exception
perfectly styled. It contains moronic errors like swapping variables around
(because these programmers do not know how to use the type system), it
contains 5 unit tests for every single little function, because that increases
the lines of code metric that is so universally used. And the reason for half
the lines of good is "good practices".

Here's how you recognize good code : firstly it is not possible to shorten it
without causing a MAJOR disaster in the readability area. Every concern is
properly separated out into it's own pieces of code, and aside from the
(short) main function there is very little single-purpose code. The typing
system is used well. Prices and amounts are NOT the same data type, for
example, and cannot be obviously switched. Unit tests exist for core
algorithmic pieces only and other than that there are system tests that
confront the code with real-world situations while running almost the entire
program, ideally under heavy load with half the backend unresponsive, and has
a statement that says the test fails if it takes longer than 1/10th of a
second. It implicitly follows and beautifully implements a design document
that is not written in word, but in a 10 to 15 line comment on top of the
file/class.

Note the issues : 1) hardcoding things 2) naming mistakes

Both of the issues you complain about can be fixed mechanically or with
absolutely minimal supervision through refactoring. Yet next to all the
comments below suggest DOUBLING the manual effort needed to get code into the
repository. While automated fixing might be problemating, writing linters that
detect these problems is trivial.

Unless of course the problem is that you feel that you need to fix how others
program because you "know better", but can't actually write code checkers. In
this case, why are you leading them ?

So well in that case I'd advise a slice of humble pie, and a compilers course.

> I review commits now and then and find some of these issues - but due to the
> nature and timeline of the project cannot do it for each and ever commit of
> course.

You sound like someone with an MBA. A programmer would recognize this for what
is is : a problem screaming to be automated. Code commits can be made
dependant on code checkers succeeding - just write the ones you want/need.
Can't do that ? You're not a programmer - or at least not a good one - and
stop whining about how difficult programming is - learn it first.

