Tips for Code Reviewing Juniors in a Far-East Asian work culture?

[Keep on Truckin]

Hi, Ask why? Many people will do or say things and they don't have a clue as to why they are doing or saying something. Often it's a political statement and has nothing to do with engineering. So, if you see or hear something you think is wrong; ask "Why?". Sometimes there is a factual reason and, you may learn something. Or, you may find the reason to be irrational. Then you can reply with a "What do you think if we did it this way?" and, show them a way you prefer.

Mac

[megaadam]

There might be a cultural difference. Ok, there IS one. But focusing the entire line of reasoning on the premise, is suboptimal, methinx. I am 100% sure that there are software companies in the US where there are ZERO code-reviews. Introducing code-reviews in such a company [whether it is in Kentucky or Kyoto] is not an overnight process. #1. You need to put some hard work in and really demonstrate to everyone that you are as great as you are. You need need to crack some tough nuts to gain some street cred. #2. You need people to not only tolerate, but also appreciate reviews. Start with management above you. Then make sure the juniors get it too. The idea of code-reviews needs to be absorbed into many levels of your organisation. #3. Assuming that everybody doesn't suck, engage the othe smart seniors, into doing reviews. And finally take a close look at what you really want...

Philip Laureano wrote:

managed to land a fairly-comfortable job

Just my two cents

_____________________________________ Action without thought is not action Action without emotion is not life

[Lost User]

Maybe show them how they can do even better?

Visit http://www.notreadytogiveup.com/[^] and do something special today.

[CurtainDog]

Ok, I'm speaking from a position of complete ignorance here, but would it be possible to seed good coding and working practices in a couple of the more receptive people and just let the others pick it up by osmosis(it's my favourite way to learn things)?

[Gary Wheeler]

Member 4475214 wrote:

Even today one of my shift staff didn't want to come in because they had a migraine - I told them I had a bottle of Panadol in my office drawer for my frequent migraines

Assuming your employee had a genuine migraine, I'm glad I don't work for you. If you told me that when I had one, I'd quit. You don't have frequent migraines; you've got minor tension headaches. If you did have them, you'd know that Panadol (common asprin) doesn't do sh!t for a migraine. I'd love to have you in my head for an hour with one of mine. You'd run away mewling and crying :mad:.

Software Zen: delete this;

[Brad Stiles]

Philip Laureano wrote:

What's the most diplomatic way to tell them that their code well...sucks?

If you do code reviews, and you find a problematic piece of code, ask them politely why they did it that way. What factors lead them to use that approach? What other approaches did they consider, and why were they rejected? At some point, ask them if they'd considered the approach you think is the correct one. If you can do this without looking at any one person's specific example of a problem, it will allow them to save the face that seems so important. Of course you might also find that your approach isn't the best way, either. :) If you find the same problems over and over again, put together a session on that class of problem, and some practices for dealing with it, without pointing to any individual's code. In fact, avoid pointing fingers if you can avoid it at all. Most people are much more accepting of constructive advice if they don't think you are talking to them specifically, or all by themselves. Brad

[Brad Stiles]

Gary Wheeler wrote:

You don't have frequent migraines; [...] You'd run away mewling and crying

Hah! I had much the same thought when I read that! My wife knows what to do when I get one of mine; darken the room, cool cloth on the forehead, and everybody out of the house. :) Fortunately, since finding a doctor who was more interested in solving the problem than medicating the crap out of me, we've identified a couple of triggers, and I avoid those like the plague now. The incidence of migraines has gone down by about 300% over the last three years.

[Gary Wheeler]

Brad Stiles wrote:

darken the room, cool cloth on the forehead, and everybody out of the house.

Same here. Dark, quiet, ice pack for the head. The funny thing about the ice pack is, it gives me a physical sensation in that part of my head that I can pay attention to instead of the pain.

Brad Stiles wrote:

since finding a doctor who was more interested in solving the problem than medicating the crap out of me

I was lucky too. I've had them all of my life, but most doctors subscribed to the notion that men didn't get migraines, and they should just suck it up and deal with it. 15 years ago, my doctor helped diagnose what was going on. She gave me good drugs for dealing with actual migraines, and helped me learn how to identify my triggers and recognizing onset more quickly. I've slowly but surely gotten better at it. I've only had a couple bad ones in the last three years, through a combination of avoiding triggers and medicating early. I'm at the point now where I can tell I'm getting one 12-24 hours in advance. I've found if I can take something for it early enough (before it gets to the "just-kill-me" stage), I'll stave the worst of it off.

Software Zen: delete this;

[CDMTJX]

I've found people in the US not liking code reviews either. You need to have them with the program that they'll be revivewed, and buy in from management that reviews will happen and results applied. How to do the reviews, and deliver results could maybe be worked out like others suggest, with your boss or someone local. There can be real cultural differences. I took an internal course at my old company about US vs Indian cultures. How both cultures think very differently than each other in the workplace...

[HydroKirby]

Philip Laureano wrote:

What's the most diplomatic way to tell them that their code well...sucks?

I suggest not even trying. Cultural ideals are embedded into them when they grow up and will be hell to break. Any suggestions you give have the possibility of being seen as heretical or critical. Rather, see if you can work around their "flawed outlook". But I understand that your job would be to tell them where stuff isn't working well. So do it as sparingly as you can. Try ordering the possible problems in the code in terms of how lethal and commonly encountered they are. Then confront them one-on-one with the biggest and baddest things first. If they have multiple issues, see if you could postpone telling them about the others things for later. Pointing out too many things may give them the feeling they are incompetent or that you are cruel. The hardest part is lessening the blow of your bad news. Don't be direct in pointing out problems; perhaps you could ask them a question that leads them towards the flaw (ex: "What if the variable was assigned this value?"). Be as discreet as possible; don't let the co-workers know or even get the hint that the person you talked to was "scolded". The person you talk to will be nervous if anyone else knew that you gave him/her advice. Lastly, see if you can resolve the coding problem in terms that even the coder would prefer. The culture is oriented towards everyone as a whole, right? So it would also be better in their eyes if the final product was more user-friendly and/or syntactically understandable (for fellow readers). While you may not like the idea (I know I wouldn't), I suggest letting all code that works slide by regardless of how poorly it was constructed. There's a good chance that they'll code that way for the rest of their life.

[Stuart Rubin]

You need to figure out a way to make the reviews as objective as possible. Maybe you can make a checklist (which may be largely based on mistakes that they have made in the past) and have them fill it out. You can use books like "Code Complete" to make checklists too. (In fact, this book has checklists at the end of the chapters.) Of course, that will only get you so far, but it might be a way to "ease" into more in-depth discussions of quality. Somehow you need to get the message across that criticism is actually good for everyone and everyone, especially Junior team members, are EXPECTED to make mistakes. You can point to statistics about bugs per line-of-code to get this point across and challenge them to beat the stats. Maybe you can "pay" (points, credits, time-off) for every improvement (bug fix, style, comments) made in a review. In the mean time, I'll just tear to shreds my American colleagues in reviews; they can take it! Good luck, dude! Stuart

[Dan Neely]

Stuart Rubin wrote:

In the mean time, I'll just tear to shreds my American colleagues in reviews; they can take it! Good luck, dude!

*sigh* I've been trying to get my project to do something like this since I was hired. I'd much prefer it over Learn-By-Blowing-Up-In-My-Face. It's always the same thing "That would be a good idea, but we don't have the money. Maybe next revision." The closest I got was having my codebase for an app ran through automated tools a coworker had from a project that was abruptly canceled following the customer being bought out. He found new regular coverage before having time for anything more in depth. :doh:

Today's lesson is brought to you by the word "niggardly". Remember kids, don't attribute to racism what can be explained by Scandinavian language roots. -- Robert Royall

[Stuart Rubin]

There's the rub; it's well known that code reviews SAVE money, not cost it! Still, it IS hard to get people to implement this kind of rigor. My thoughts on checklists, procedures, etc. for code reviews is that a bad (or at least inadequate) process is better than none at all. With respect to the original poster, maybe he can have his team collectively generate a set of coding standards, best practices, etc. Then, everyone will have some ownership in the standards and will understand their need better.

[Dan Neely]

You know that. I know that. Over the long term I think my project management knows that. With me as a solo dev on one app (in reasonably good shape (I think)), and a second person as an almost solo dev on the other (a known intern spawned codethulu), I think the hangup is the spinup time needed to get someone else familiar with the appropriate codebase, combined with prayers that the desert oasis contract'll finally come in and we'll be able to rewrite the demon properly in something that is not excel VBA. X|

Today's lesson is brought to you by the word "niggardly". Remember kids, don't attribute to racism what can be explained by Scandinavian language roots. -- Robert Royall

[Roger Wright]

Philip Laureano wrote:

What's the most diplomatic way to tell them that their code well...sucks?

First you have to beat them in hand-to-hand combat; then they'll respect you and listen to your opinions about code. As their sensei you would have the right to instruct them, with no one losing face.

"A Journey of a Thousand Rest Stops Begins with a Single Movement"

[cpkilekofp]

When going through their code, remember this: there was a time when you didn't know it all, either. This problem happens with all junior programmers. The "Your code sucks!" approach, well, sucks - and I say this as an American with deep pre-Revolutionary roots. You have to teach them, and the traditional northern Chinese approach of beat-'em-'til-they-get-it-right fosters either learned helplessness or rebellion. Instead, walk them through it. Show them the problems by example. Use pencil-and-paper modeling, as this always makes the problem spots pop out. With the six-level try-catch block, pick a line deep within the nested blocks and show what happens when an exception occurs there, then ask the programmer how they could handle the exception in a better way. If they offer a solution that isn't better, go back and show what happens. Let the code and its consequences speak for itself. This is the ONLY way, and it works all the time. Your opinion about code is only accepted after someone becomes convinced that if they hear your opinion, you can demonstrate it really isn't opinion - so skip the intervening step and show them the facts. The facts always speak for themselves. And, once more, remember: just because you were allowed to tell someone their code sucked in America, that didn't make it the right thing to do there, either.

[firegryphon]

I can only offer suggestions from the Japanese perspective, as that is the only Asian culture I have dealt with. With that caveat in mind, it is important to remember that understating is the norm. One person described to me that something would be difficult with a very small facial gesture, and what he really meant is that it may well be impossible. In that culture, if you are uchi, you have a better chance to constructively advice someone. In the end, with my group, we were fortunate to have all very experienced engineers that were heavily briefed on the expectations of working with an American company. As we were all briefed on Japanese culture, we were able to successfully meet in the middle somewhere. We watched out for when they understated something and asked them and they worked at not freaking out if we tried to rush their process along due to our own deadlines. So the key advice I can give is understate how bad you think it is and try to lead them to the answer with hints without saying it outright.

[Gzep]

how about test driven programming? if they write a test for each case that they code, and the test is valid, then they will see where the code fails... and fix it without losing face. even if someone else writes the test, the programmer could run them, see the results, and fix the problem.

[EinA]

If you told me that when I had one, I'd quit.

Problem solved! :-)

You don't have frequent migraines; you've got minor tension headaches.

Oh, we have a Doctor in the house!

If you did have them, you'd know that Panadol (common asprin) doesn't do sh!t for a migraine.

Oh, maybe not. Panadol does not contain "asprin" OR aspirin. While you are checking this in your medical textbooks, Google "panadol migraine" - maybe we have found a miracle cure for you? :-)

I'd love to have you in my head for an hour with one of mine. You'd run away mewling and crying

Symptoms for my staff include checking the calendar to see whether they have any paid sick days unused. Coincidentally migraines also only occcur on Mondays or Fridays. But seriously folks, some of us are trying to run businesses in Asia. Certainly the attitude towards work in some of these countries is that it takes priority behind family, friends, social life, etc. Makes it difficult to compete globally & in the country I happen to be in, I venture to say they will never be competitive while they think the world owes them a living. All we can hope for is that the Generation Y workers in those more developed countries Twitter themselves out of the competition.

[Snowman58]

I remember the "that will be difficult" comment! If accompanied by a sucking noise it really meant "no f***ing way you stupid genetically defective round eye". The more junior the person telling you, the worse the insult. Seiously, the concept of "face" is all important in most asian countries. If a negative comment is percieved, it causes a loss of face. So circular, understated approaches are needed.