CODEWITHSUNDEEP
c#vs-web-site-project
hwmaat
hwmaat

Reputation: 69

GC.Collect() in Dispose in simple class

I have inherited a kind of ancient website project in c#. It originates from 2003 This has all over the place simple classes defined that inherit from IDisposible and implement a Dispose() method where GC.Collect() is called as below:

public class ProjectAutostart : IDisposable
{
    public void Dispose()
    {
        Dispose(true);
        GC.Collect();
    }
    protected virtual void Dispose(bool disposing) { }
    private Int32 _id;
    private Int32 _stepid;
    private Int64 _stepcounter;

    public Int32 ID
    {
        set { _id = value; }
        get { return _id; }
    }
    public Int32 StepID
    {
        set { _stepid = value; }
        get { return _stepid; }
    }
    public Int64 StepCounter
    {
        set { _stepcounter = value; }
        get { return _stepcounter; }
    }
}

These classes are called like:

List<Projects.ProjectAutostart> ProjectList = DataLayer.Projects.getProjectAutoStart();

Which ends up in:

public static List<Projects.ProjectAutostart> getProjectAutoStart()
{
    List<Projects.ProjectAutostart> Projects = new List<Projects.ProjectAutostart>();
    DataTable DataTable = SQL.DataTable("getProjectAutoStart", null);
    foreach (DataRow dt in DataTable.Rows)
    {
        Projects.Add(new ProjectAutostart { ID = Convert.ToInt32(dt["projectid"]), StepID = Convert.ToInt32(dt["stepid"]), StepCounter = Convert.ToInt32(e["autostartstepcounter"]) });
    }
    DataTable.Dispose();
    return Projects;
}

I'm not experienced with this type of projects, I'm totally into the .net core restfull area so this code is strange to me. Asides from the total weird way of implementing, These GC.Collect() and that Dispose() feel totally useless since it is managed code and it are simple class without any coding executing. Why would someone put that Dispose and GC.Collect() in there? Should I just remove it?

Upvotes: 0

Views: 150

Answers (1)

PMF
PMF

Reputation: 17288

As already noted in the comments, GC.Collect() in the Dispose method is completely superfluous here. Maybe they got the dispose pattern wrong in the first place.

The recommended implementation for the dispose pattern is as follows:

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this); // sic!
    }
    protected virtual void Dispose(bool disposing) 
    {
    }

However, since your ProjectAutostart class is a pure DTO and does not have any disposable fields (and any derived classes probably don't, either), you don't even need to implement dispose here. Just remove both Dispose methods as well as the interface declaration. What you do need to dispose is the DataTable object, but that's properly done already.

Upvotes: 1

Related Questions