Assignment in method call bad practice?

Question

This is my first question to Stackoverflow, although I have been a consumer for many years. Please forgive me if I break rules. That is certainly not my intent. I have strenuously reviewed the rules and believe I am within the bounds of acceptability. However, I would ask that you kindly point out usage errors, if they are present, so that I may in future be more compliant.

I teach programming to high school students. This semester we are doing C#. Last week we were studying recursion. I assigned several classic problems that can be solved with recursion, one of them being exponentiation.

One of my students submitted the following code as a solution for exponentiation using recursion (he did give me permission to post it here). The assignment in the method call gave me the willies, but when I told him that it was bad practice, he protested, saying that "it works", that it "made sense" to him, and that he "does it all the time".

static void Recur(int n1, int n2, int n3) 
{ 
   if (n2 > 0) 
   { 
      Recur(n1, n2 - 1, n3 *= n1);   // this is the line in question
   } 
   else 
   { 
      Console.WriteLine("The number calculated recursively is: {0}", n3); 
   } 
} 

I have had a hard time coming up with something concrete to tell my student about why making an assignment in a method call is bad practice in general, other than 1) the possibility of unintended side effects, and 2) the difficulty of maintenance.

I have searched online with every phrase I can construct concerning this question, but have come up pretty empty-handed. I did see a reference to a book called "Clean Code" by Robert C. Martin that I don't own.

My relationship with my students is somewhat like that of a parent. They sometimes don't put a lot of stock in what I say unless I can corroborate it with an independent source. If I could point to a definitive statement about putting assignments in a method call my student would be more inclined to stop this annoying habit.

Does this usage bother anyone else? Am I expecting too much in wanting him to change the way he's been doing things? He's 15, but with a promising future ahead of him. He's one of those students who just "gets it". I don't want him to develop bad practices.

Thank you for your time and input.

Answer 1

There are quite a few things improvable in the posted code:

1. Be consistent

   Why isn't the call Recur(n1, n2 =- 1, n3 *= n1)? Why is n3 treated differently? This can cause confusion when reviewing/mantaining code.

2. Dont do unnecessary or superfluous work

   Is n3 ever used after Recur(n1, n2 - 1, n3 *= n1)? No? Then why waste time assigning a value that is never used?

3. It's considered good practice to not mutate method arguments (unless they are passed in by reference of course). Why? Because it makes debugging and understanding how the code works much harder; having the initial conditions mutate as the method executes makes it much harder to track possible bugs, optimizations, improvements, etc.

All that said, I avoid these kind of patterns because I have really bad memory:

var i = 0;
Foo(i += 1);
Bar(i);

What is passed into Foo? Was it 0 or was it 1? I never remember and I have to look it up everytime. Chances are that whoever reviews this code can have the same problem. These clever tricks don't make the code work any faster or better and it avoids a grand total of one code line... not worth it.