Send an object to a method, and return this object C#

Question

I'm currently working with C#, but this question is the same with all OO languages, I think.

I have a method, located in my "ServersList" class. In this method, I fill a ServersList with Deserialization.

Here's the method :

public ServersList fillList(ServersList serversList)
{
    //ServersList serversList = new ServersList();
    XmlSerializer serializer = new XmlSerializer(typeof(BusinessObjects.ServersList));

    using (StreamReader reader = new StreamReader("Resources/XMLServers.xml"))
    {
        serversList = (BusinessObjects.ServersList)serializer.Deserialize(reader);
    }

    return serversList;
}

I don't know if it's better to create a ServersList in Main.cs, call my method with this SereversList as parameter and return the result, or call the function without parameter and create the ServersList directly inside it (like I do in the commented line)

Sorry if my question seems dumb or something, I'm pretty new to OOP and I really don't know which method is the best. Thank you for your help !

Answer 1

I see no purpose in passing in an empty list, since you don't use it's value anyway. It would only confuse future reader an maintainer, especially on the user side. And I would make it static too. Compare this:

var serversList = new ServersList();
serversList = serversList.fillList(serversList);

To this:

var serversList = ServersList.LoadFromResources("Resources/XMLServers.xml");

Personally, I would rather write it like this:

public static ServersList LoadList(string _FileName)
    {
        using (StreamReader reader = new StreamReader(_FileName))
        {
            return (BusinessObjects.ServersList)serializer.Deserialize(reader);
        }
    }

By the way, it would be a classic example of Factory pattern.

Answer 2

If you need it only inside the method, create and initialize it within a method. Object'll be held on heap anyway, while reference to it will be held either on a heap (while declared in a class) or on a stack (if declared within method body). It's irrelevant here.

There's absolutely no reason to pass a reference's copy to a method without intending to read from it and modify it inside.

Answer 3

Looking at this line:

serversList = (BusinessObjects.ServersList)serializer.Deserialize(reader);

The method Deserialize already creates a new list and replaces the list you have. This renders the parameter useless and also the commented line.

My suggestion is to change the method to:

public ServersList CreateServersList();

And the implementation:

    XmlSerializer serializer = new XmlSerializer(typeof(BusinessObjects.ServersList));

    using (StreamReader reader = new StreamReader("Resources/XMLServers.xml"))
    {
        return (BusinessObjects.ServersList)serializer.Deserialize(reader);
    }

This way the list created by the deserialization is directly returned to the caller.

Answer 4

Befor call Deserialize in you'r method, serversList point on method parameter reserved memory but when made Deserialize, serversList point on new ServersList object that created in serializer.Deserialize() method. then you don't need to get him in function's parameter.

Answer 5

Not only that you don't need to pass this as a parameter, but it's actually a very bad idea. It implies that the method is doing something with the initial list, possibly modifying its contents, while it's actually creating a completely new instance and returning it.

Consider this code, for example:

public string DoStuff(string text)
{
    text = "Something else";
    return text;
}

You can easily call this method without even using the return value:

var text = "My text";

// this compiles
DoStuff(text);

// but the original variable was not changed
Console.WriteLine(text);

Consider, however, at least removing the hardcoded string and fetching it from a, say, config file:

public static ServersList LoadServersList()
{
    var serializer = new XmlSerializer(typeof(ServersList));
    var configFile = ConfigurationManager.AppSettings["ServerListPath"];
    using (var reader = new StreamReader(configFile))
    {
        return (ServersList)serializer.Deserialize(reader);
    }
}