Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I've started learning .NET with MVC 4 in C# and I built a basic application to display records from a MySQL database. It's incredibly basic, but it's the best I could come up with being a complete beginner to C# and all.

Most of the programming I do is with Ruby on Rails and I tried to carry over some of the design patterns that I use there. I don't know the .NET conventions so if you see anything that should be changed please let me know.

Also, I don't fully understand how to find records with LINQ and pass them to the view. I ended up getting lucky and finding an article that somewhat explained it, but I don't have a firm grasp on the subject. If you know of any tutorials or articles that could shed some light on the subject I'd be very grateful.

P.S. The naming conventions for my entities and models are wrong. It's a long story, basically it took me a while to connect to my MySQL database and when I generated them I didn't care how they were named.

Here's the code I would like ya'll to look at.

RouteConfig.cs

routes.MapRoute(
    name: "root",
    url: "",
    defaults: new { controller = "Authors", action = "Index" }
);

routes.MapRoute(
    name: "author_books",
    url: "authors/{author_id}/books",
    defaults: new { controller = "Authors", action = "Books" }
);

AuthorsController.cs

public class AuthorsController : Controller {

    bookisticsEntities be = new bookisticsEntities();
    //
    // GET: /Authors/

    public ActionResult Index() {
       return View(be.authors.ToList());
    }

    public ActionResult Books(int author_id = 0) {
        author aut = be.authors.Find(author_id);

        var books = from b in be.books
                    join a in be.authors
                    on b.author_id equals a.id
                    where b.author_id == author_id
                    select b;

        ViewBag.author_name = aut.name;
        return View(books.ToList());
    }
}

I've omitted the view files because I pretty much left them at default after they were generated.

share|improve this question
add comment

1 Answer 1

up vote 2 down vote accepted

Dependency injection

Use dependency injection everywhere you can

public class AuthorsController : Controller
{
    private readonly bookisticsEntities _booksEntities;

    public AuthorsController(bookisticsEntities entities)
    {
        _booksEntities = entities;
    }

    public ActionResult Index()
    {
        //all authors on one page can be slow, paging?
        //.ToList() is useless here
       return View(be.authors);
    }

    //default value is not neccessary becouse 0 is an invalid value
    public ActionResult Books(int author_id)
    {
        var author = _booksEntities.authors.SingleOrDefault(a => a.id == author_id);

        if (author == null)
        {
            return HttpNotFound();
        }

        /*var books = from b in be.books
                    join a in be.authors
                    on b.author_id equals a.id
                    where b.author_id == author_id
                    select b;

        ViewBag.author_name = aut.name;*/

        //pass the author to the view and in the view you can iterate on the author.Books collection
        //if the foreign keys are correct in the database
        //or you setted everything correctly in code first approach
        return View(author);
    }
}

Now the controller expects a bookisticsEntities instance as constructor parameter but the default controller factory in the MVC framework only can use a parameterless constructor so you need a solution for this; use a dependency injection container like Ninject! It's easy to use available through Nuget.

N-tier

It seams to me your application missing one or two (maybe three) application level becouse currently you managing the database directly which is not a best practise. You have to build at leas one level between the MVC and the dataaccess tier which is called business layer (or two: a service layer and under that a business layer). In this way you controller will expect one or more different type of object instances but not your database model!

If you are done whith this you can use simplified models in your application, for example in the Books action method you can use a model which can contain the author's name, identifier and the books also (books can be simplified also).

Learn Entity Framework

Read a lot about how EF works. :)

share|improve this answer
    
Thank you for your suggestions. I'm definitely interested in implementing DI and Ninject looks like a really cool project. However, I couldn't get it work out of the box. I added it through Nuget, and I added a constructor to my class, but I'm getting an error about not having a parameterless constructor. After a couple of google searches I wasn't able to find a direct answer, do you have any ideas on what I should do? –  Baylor Rae' Feb 8 '13 at 15:14
    
@BaylorRae' You need to ensure that the binding is setup so that the Ninject solution knows what class to create when the controller class is instantiated –  dreza Feb 9 '13 at 2:51
    
Add to the project the Ninject MVC3 extension and then you will have a NinjectWebCommon class where you can set up your bindings. This is not required you can use it for example in the Applocation_Start by creating a StandardKerel, setting up the bindings and then with a small adapter class register it as a DependencyResolver (DependencyResolver.SetResolver) and you are done. –  Peter Kiss Feb 9 '13 at 6:18
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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