Assignment in method call bad practice?

 

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.

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

  • 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.

  • 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?

  • 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.

    The bad code is this one: n3 *= n1 Your student doesn't see any problem because method parameters are not final . Just define them final and you will see this peace of code will cause a compile error.

    Why should one use final ? Because this makes sure the method parameters will not be used for any other purposes inside the method. This makes the code less error prone, more reliable. Using one variably for one purpose makes also the code easier to understand by others which is important in the industry.

    And as already pointed by others, assigning a value to n3 makes not sense because further on variable n3 is not used in the code.

    His statement that this "makes sense to him" shows that he does not about software development in the real life, in the industry - starting from small companies and to Google or Facebook. One of the requirements is that the code should be maintainable. This peace of code is not.

    Regarding "with a promising future ahead of him": How do you know that? Did he win any programming competition? His argument that he "does it all the time" is pretty weak. Ask him why does he do it? Give him a small research about the code quality. May be this will help him to understand software development a little bit.

    链接地址: http://www.djcxy.com/p/41364.html

    上一篇: Windows：以C ++获取函数地址

    下一篇: 方法调用不好的做法中的作业？