Aspnetcore.docs: Mismatch between code samples and information.

Created on 16 Mar 2018  路  9Comments  路  Source: dotnet/AspNetCore.Docs

https://docs.microsoft.com/en-us/aspnet/mvc/overview/older-versions/getting-started-with-ef-5-using-mvc-4/implementing-the-repository-and-unit-of-work-patterns-in-an-asp-net-mvc-application#implement-a-generic-repository-and-a-unit-of-work-class

The unit of work class has this code:

 public GenericRepository<Department> DepartmentRepository
        {
            get
            {
                return this.departmentRepository ?? new GenericRepository<Department>(context);
            }
        }

and a bit lower in the code there is this information:

Each repository property checks whether the repository already exists. If not, it instantiates the repository, passing in the context instance. As a result, all repositories share the same context instance.

public GenericRepository<Department> DepartmentRepository
{
    get
    {
        if (this.departmentRepository == null)
        {
            this.departmentRepository = new GenericRepository<Department>(context);
        }
        return departmentRepository;
    }
}

Actually this line of code is wrong:
get { eturn this.departmentRepository ?? new GenericRepository<Department>(context); }

as this returns always a new instance of GenericRepository in case the field is null!

Most helpful comment

I understand now. I investigated and discovered that the code you're talking about was the result of a commit by @rayden84 last October. I will undo the commit. Thanks for pointing that out. (Rick is right about lack of resources -- I don't currently work on ASP.NET docs anymore but am squeezing it into my schedule anyway.)

All 9 comments

@wstaelens thanks for the information. Unfortunately, we don't have the resources to update this document.

so it is just closed? oh boy.
Next time I think I will not report another issue if this is how it is handled.

Is there a reason you can't review the ASP.NET Core version at https://docs.microsoft.com/en-us/aspnet/core/data/ef-mvc/advanced ? Even that is scheduled to be removed. Advanced EF topics will all be documented at https://docs.microsoft.com/en-us/ef/core/

The RP version stops at step 8 for that reason.

You could make a PR and fix the problem if you like.

I just reported an issue with the code in some documentation so that other users don't use the same mistake. I don't know how you are internally organizing and planning things at Microsoft. All I did is just reporting a code issue, hoping it would be fixed and it would help others as these websites are linked and used in a lot of places and forums (some teachers even print it out or copy/paste it in slides).
But if that is just closed, then I don't know what I'm doing here anymore.

It feels like reporting issues in the past in Microsoft Connect where the team just closes it as "not important enough".

I need a coffee 鈽曪笍 馃槥

cc @tdykstra

@wstaelens I don't follow what is wrong here:

Actually this line of code is wrong:
get { eturn this.departmentRepository ?? new GenericRepository<Department>(context); }
as this returns always a new instance of GenericRepository in case the field is null!

What is shared is the context, and that is passed in to the new instance, as described above:

Each repository property checks whether the repository already exists. If not, it instantiates the repository, passing in the context instance. As a result, all repositories share the same context instance.

get { return this.departmentRepository ?? new GenericRepository<Department>(context); }.
in case departmentRepository is null, it returns a new instance of Department. The deparmentRepository field stays null. So the next time it will do the same code "new GenericRepository<..." again returning a new instance. the next time again, and again. because the "this.departmentRepository" is never set.

This code does set the departmentRepository field when null.

    {
        if (this.departmentRepository == null)
        {
            this.departmentRepository = new GenericRepository<Department>(context);
        }
        return departmentRepository; 

The information shows a code sample with the ?? part but explains in detail with another code part (if ..== null)...
both codes have similar result but don't act the same.

I understand now. I investigated and discovered that the code you're talking about was the result of a commit by @rayden84 last October. I will undo the commit. Thanks for pointing that out. (Rick is right about lack of resources -- I don't currently work on ASP.NET docs anymore but am squeezing it into my schedule anyway.)

@tdykstra Thanks for taking care of this, Tom!

Was this page helpful?
0 / 5 - 0 ratings