6
\$\begingroup\$

I have simple database with many to many relations.

enter image description here

I wrote simple method GetAuthors()

[HttpGet]
public JsonResult GetAuthors()
{
    var books = db.Authors
        .Where(c => c.Name == "FirstAuthor")
        .Include(b => b.AuthorsBooks)
        .Select(p => new
        {
            author = p.Name,
            books = p.AuthorsBooks.Select(z => z.Book.Name)
        })
        .ToList();

    return new JsonResult(books);
}

that returns expected result.

[{"author":"FirstAuthor","books":["GoodBook1","GoodBook2","GoodBook3"]}]

The main question how to simplify LINQ in funtion GetAuthors() currently it looks over complicated. Maybe exist other way to simplify with StoredProcedure? how to improve it?

Controller code:

namespace DataFromMSSQL.Controllers
{
    [Route("api/[controller]")]
    [ApiController]
    public class ValuesController : ControllerBase
    {
        private readonly StrangeContext db;

        public ValuesController(StrangeContext context)
        {
            db = context;
        }

        [HttpGet]
        public JsonResult GetAuthors()
        {
            var books = db.Authors
                .Where(c => c.Name == "FirstAuthor")
                .Include(b => b.AuthorsBooks)
                .Select(p => new
                {
                    author = p.Name,
                    books = p.AuthorsBooks.Select(z => z.Book.Name)
                })
                .ToList();

            return new JsonResult(books);
        }
    }
}

Data base script

/****** Object:  Table [dbo].[Authors]    Script Date: 5/11/2019 10:24:25 AM ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
CREATE TABLE [dbo].[Authors](
    [Id] [int] IDENTITY(1,1) NOT NULL,
    [Name] [nvarchar](30) NULL,
 CONSTRAINT [PK_Versions] PRIMARY KEY CLUSTERED 
(
    [Id] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY]
GO
/****** Object:  Table [dbo].[AuthorsBooks]    Script Date: 5/11/2019 10:24:25 AM ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
CREATE TABLE [dbo].[AuthorsBooks](
    [Id] [int] IDENTITY(1,1) NOT NULL,
    [AuthorId] [int] NOT NULL,
    [BookId] [int] NOT NULL,
PRIMARY KEY CLUSTERED 
(
    [Id] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY]
GO
/****** Object:  Table [dbo].[Books]    Script Date: 5/11/2019 10:24:25 AM ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
CREATE TABLE [dbo].[Books](
    [Id] [int] NOT NULL,
    [Name] [nvarchar](30) NULL,
PRIMARY KEY CLUSTERED 
(
    [Id] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY]
GO
SET IDENTITY_INSERT [dbo].[Authors] ON 

INSERT [dbo].[Authors] ([Id], [Name]) VALUES (1, N'FirstAuthor')
INSERT [dbo].[Authors] ([Id], [Name]) VALUES (2, N'SecondAuthor')
SET IDENTITY_INSERT [dbo].[Authors] OFF
SET IDENTITY_INSERT [dbo].[AuthorsBooks] ON 

INSERT [dbo].[AuthorsBooks] ([Id], [AuthorId], [BookId]) VALUES (1, 1, 1)
INSERT [dbo].[AuthorsBooks] ([Id], [AuthorId], [BookId]) VALUES (2, 1, 2)
INSERT [dbo].[AuthorsBooks] ([Id], [AuthorId], [BookId]) VALUES (3, 1, 3)
INSERT [dbo].[AuthorsBooks] ([Id], [AuthorId], [BookId]) VALUES (4, 2, 1)
INSERT [dbo].[AuthorsBooks] ([Id], [AuthorId], [BookId]) VALUES (5, 2, 3)
SET IDENTITY_INSERT [dbo].[AuthorsBooks] OFF
INSERT [dbo].[Books] ([Id], [Name]) VALUES (1, N'GoodBook1')
INSERT [dbo].[Books] ([Id], [Name]) VALUES (2, N'GoodBook2')
INSERT [dbo].[Books] ([Id], [Name]) VALUES (3, N'GoodBook3')
ALTER TABLE [dbo].[AuthorsBooks]  WITH CHECK ADD FOREIGN KEY([AuthorId])
REFERENCES [dbo].[Authors] ([Id])
GO
ALTER TABLE [dbo].[AuthorsBooks]  WITH CHECK ADD FOREIGN KEY([BookId])
REFERENCES [dbo].[Books] ([Id])
GO
/****** Object:  StoredProcedure [dbo].[GetBooksByAuthor]    Script Date: 5/11/2019 10:24:26 AM ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
CREATE PROCEDURE [dbo].[GetBooksByAuthor]
AS
Select authors.Name, books.Name from dbo.Authors authors
INNER JOIN dbo.AuthorsBooks authorsBooks on authors.Id=authorsBooks.AuthorId
INNER JOIN dbo.Books books on books.Id=authorsBooks.BookId
GO
USE [master]
GO
ALTER DATABASE [Strange] SET  READ_WRITE 
GO
\$\endgroup\$
2
\$\begingroup\$

I don't see how to optimize the linq-query, and it doesn't look complicated to me. It's a method that returns authors with a specific name. That is of course of very little use, so the first thing to do, is to provide the name to search for as a parameter to the method:

public JsonResult GetAuthors(string name) {... .Where(b => b.Name == name) ...}

The generated Json object is also not very "helpful" to the client in that it only contains names of the authors and their books. Returned to the client and displayed in a list it's OK, but when the user clicks on one of the authors or books, I suppose, you'll want to fetch more information about the selected object from the server. Therefore you should provide the Id for both authors and books, in order to return that to the server, when the user clicks on an item - which on the server can be handled by:

public JsonResult GetAuthor(int id) {...}

and

public JsonResult GetBook(int id) {...}

So I would change your method to:

    [HttpGet]
    public JsonResult GetAuthors(string name)
    {
        var books = db.Authors
            .Where(c => c.Name == name)
            .Include(b => b.AuthorsBooks)
            .Select(p => new
            {
                id = p.Id,
                name = p.Name,
                books = p.AuthorsBooks.Select(z => new { id = z.Id, name = z.Book.Name })
            })
            .ToList();

        return new JsonResult(books);
    }

\$\endgroup\$
  • \$\begingroup\$ The .Include() shouldn't be necessary since there is a projection. \$\endgroup\$ – Brad M May 13 at 13:58
1
\$\begingroup\$

Not linked directly to your question but here are some things I would change:

I'd name my tables slightly different: Author, Book and then AuthorsBooks. And the ID column of each of the two main tables would be AuthorId and BookId.

Please note that this is highly debatable and perhaps even just a matter of personal prefference, if you do a search here and stackoverflow you'll see what I mean.

From my point of view a datatable represents a definition of a single entity (Author), the rows inside then represent many of those entities.

ID column naming AuthorId and BookId is more of a DB thing where SQL statements become more readable in JOIN scenarios, if you do a join on these two tables then you must alias the columns in results (Author.Id As AuthorId and Book.Id As BookId) because SQL doesn't allow column results with same names in complex statements.

The many-to-many table doesn't need an ID column. You generally need and ID column if you plan on deleting/updating or referencing rows. In a many-to-many link table you're suppose to have only one combination of AuthorId and BookId so there's no need for deletions and updates also referencing by row ID is not needed in this table.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.