Is it bad practice to modify the variable within a method?

Question

Which method style is better? Is it generally bad practice to modify the variable within a method?

public class Person
{
   public string Name { get; set;}
}

//Style 1
public void App()
{
    Person p = new Person();
    p.Name = GetName();
}
public string GetName()
{
   return "daniel";
}

//Style 2
public void App()
{
    Person p = new Person();
    LoadName(p)
}
public void LoadName(Person p)
{
   p.Name = "daniel";
}

Ron Harlev · Answer 1 · 2011-04-15T05:24:47.607

4

I think the code is more clear and readable when methods don't change objects passed. Especially internal fields of passed object.
This might be needed sometimes. But in general I would avoid it.

Updated based on comment (good point)

edited Apr 15 '11 at 05:24

answered Apr 15 '11 at 05:06

Ron Harlev

16,227
24
89
132

2

It's worth mentioning that nothing in either style demonstrated is passed by reference. Reference types are used, yes. A copy of the reference is passed by value. There is a fundamental difference between that and "passing by reference" in C#. – Anthony Pegram Apr 15 '11 at 05:22

score 4 · Accepted Answer · answered Apr 15 '11 at 05:31

There are times when both styles may make sense. For example, if you're simply setting the name, then perhaps you go with the first style. Don't pass an object into a method to mutate one thing, simply retrieve the one thing. This method is now more reusable as a side benefit. Think of it like the Law of Demeter or the principle of least knowledge.

In other cases, maybe you need to do a wholesale update based on user input. If you're displaying a person's attributes and allowing the user to make modifications, maybe you pass the object into a single method so that all updates can be applied in one spot.

Either approach can be warranted at different times.

score 2 · Answer 3 · answered Apr 15 '11 at 06:11

I agree with Anthony's answer. There are times when both styles may make sense.

Also, for more readability you can add the LoadName function in person class.

public void App()
{
    Person p = new Person();
    p.LoadName(); //If you need additional data to set the Name. You can pass that as Parameter
}

score 0 · Answer 4 · edited Apr 15 '11 at 05:23

0

I agree with Ron. Although your particular example could be slightly contrived for posting reasons, I would have a public getter for Name, and a private setter. Pass the name to the constructor, and the Name property will get set there, but afterwards can no longer be modified.

For example:

public class Person
{
    public string Name { get; private set; }

    public Person( string name)
    {
        Name = name;
    }
}

edited Apr 15 '11 at 05:23

Anthony Pegram

123,721
27
225
246

answered Apr 15 '11 at 05:11

Dave

14,618
13
91
145

So if a person legally changes his or her name, perhaps gets married, this cannot be represented in your suggested object model? – Anthony Pegram Apr 15 '11 at 05:24
It could if you added a method to change the name, i would assume if you're changing the name then theres more going on than just changing the name. – Phill Apr 15 '11 at 05:28
@Phill, a method to change the name sounds suspiciously like a setter. We have idioms for that in C#. ;) More than that, the text of the answer implied post-construction immutability. A person is by his or her very nature mutable. – Anthony Pegram Apr 15 '11 at 05:36
@Anthony I was just assuming this was a contrived example. How about if I just rename the property to `FirstName`? :) – Dave Apr 15 '11 at 06:09
Then again, this does remind me of a guy in college I met that said, "Hi my name is Jim, but you can call me Drew". – Dave Apr 15 '11 at 06:10

score 0 · Answer 5 · answered Apr 15 '11 at 05:11

You are accessing the data using properties which technically is by a methods. What you are worried is property accessing iVar or internal variable. There reason why it is generally bad to allow access of iVar is because anyone can modify the variables without your knowledge or without your permission, if its through a methods (properties), you have the ability to intercept the message when it get or set, or prevent it from getting read or write, thus it is generally said to be the best practice.

Is it bad practice to modify the variable within a method?

5 Answers5