Reputation: 156
I've been reading on base controllers and I can't seem to understand the concept. I need to simplify the code in the controller because I just repeat code for nothing. I use a database to collect data for the website and build some lists to use in views. All the pages share two of the lists which i copied in the base controller. When i remove the lists in the pagescontroller I get an error that the name 'sliders' and 'partenaires' does not exist in the current context. What would be the best way to prevent repeating code?
This is my base controller :
public class BaseController : Controller
{
PIAEntities db = new PIAEntities();
public BaseController()
{
var sliders = db.Actualite.Where(a => a.Afficher).OrderByDescending(a => a.Date_publication);
var partenaires = db.Partenaire.Where(p => p.Afficher);
}
}
And this is a part of the controller that manages all my pages
public class PagesController : BaseController
{
public PagesController() : base()
{
}
PIAEntities db = new PIAEntities();
public ActionResult Activités()
{
var sliders = db.Actualite.Where(a => a.Afficher).OrderByDescending(a => a.Date_publication);
var partenaires = db.Partenaire.Where(p => p.Afficher);
var model = new ModelDeBase { Slider = sliders, Partenaire = partenaires };
return View(model);
}
public ActionResult Actualités()
{
var sliders = db.Actualite.Where(a => a.Afficher).OrderByDescending(a => a.Date_publication);
var pageActualites = db.Actualite.OrderByDescending(a => a.Date_publication).Take(10);
var donateurs = db.Donateur.Where(d => d.Afficher);
var partenaires = db.Partenaire.Where(p => p.Afficher);
var model = new ModelActualite { Slider = sliders, Partenaire = partenaires, PageActualite = pageActualites, Donateur = donateurs };
return View(model);
}
}
Solution :
Ok now it works and I ended up using this in my base controller
public class BaseController : Controller
{
PIAEntities db = new PIAEntities();
protected IEnumerable<Actualite> sliders { get; private set; }
protected IEnumerable<Partenaire> partenaires { get; private set; }
public BaseController()
{
sliders = db.Actualite.Where(a => a.Afficher).OrderByDescending(a => a.Date_publication);
partenaires = db.Partenaire.Where(p => p.Afficher);
}
}
and the Pages controller :
public class PagesController : BaseController
{
public PagesController() : base()
{
}
PIAEntities db = new PIAEntities();
public ActionResult Activités()
{
var model = new ModelDeBase { Slider = sliders, Partenaire = partenaires };
return View(model);
}
public ActionResult Actualités()
{
var pageActualites = db.Actualite.OrderByDescending(a => a.Date_publication).Take(10);
var donateurs = db.Donateur.Where(d => d.Afficher);
var model = new ModelActualite { Slider = sliders, Partenaire = partenaires, PageActualite = pageActualites, Donateur = donateurs };
return View(model);
}
}
Upvotes: 1
Views: 2498
Reputation: 218732
With your current code, you are querying Actualite's add Partenaire's in the constructor of the BaseController
and your PagesController
is inheriting from that. But in your Activities action method,you are again querying the same data ! When user requests Pages/Activites
, it is going to execute your BaseController constructor code and the code in your Activity controller. So you have duplicate code there. Also the data you are querying in the BaseController constructor is being set a variable(with local scope to the method) and not being used in any other place. One thing you can do is to keep some protected variables and assign to that. You can access these protected variables in your child classes.
public class BaseController : Controller
{
protected List<Actualite> Actualities {set;get;}
PIAEntities db = new PIAEntities();
public BaseController()
{
this.Actualities = db.Actualite.Where(a => a.Afficher)
.OrderByDescending(a => a.Date_publication).ToList();
}
}
Now in your PageController, you may simply user this.Actualites
. you do not need to query again.
public ActionResult Activités()
{
var model = new ModelDeBase { Slider = this.Actualities };
return View(model);
}
You can do the same thing for your other collection.
Since you have that code in the constructor, It will be called for any call to any controller which inherit from BaseController. But some of your action method may not need this data (but still want to user other base controller variables/methods). In that case, i suggest you move it to a service class and call it as needed.
public class PageService
{
public IEnumerable<Actualite> GetActualites()
{
return db.Actualite.Where(a => a.Afficher)
.OrderByDescending(a => a.Date_publication).ToList();
}
}
Now in your controllers, you create an object of this and call the GetActualites
method as needed.
public ActionResult Activités()
{
var s=new PageService();
var model = new ModelDeBase { Slider =s.GetActualites()};
return View(model);
}
You may go one step further and extract an interface and inject the concrete implementation using a dependency injection framework. This will help you to unit test your controllers.
Upvotes: 1
Reputation: 9749
Both of your varaibles i.e. sliders
and partenaires
are declared in the scope of BaseController
constructor, so they will not be accessible outside the constructor.
You should declare them at class level with suggestively as private
setters so that child classes will not be able to change these properties.
public class BaseController : Controller
{
PIAEntities db = new PIAEntities();
protected IEnumerable<Actualite> sliders { get; private set; }
protected IEnumerable<Paternaire> partenaires { get; private set; }
public BaseController()
{
this.sliders = db.Actualite.Where(a => a.Afficher).OrderByDescending(a => a.Date_publication);
this.partenaires = db.Partenaire.Where(p => p.Afficher);
}
}
Upvotes: 1
Reputation: 56
Add a couple of fields in your base controller to hold the values of sliders & partenaires. You can then set these in the constructor.
this._sliders = db.Actualite.Where(a => a.Afficher).OrderByDescending(a => a.Date_publication);
You should then be able to access these fields from the derived controller
Upvotes: 0
Reputation: 62488
You have to make them fields or properties in the base controller, currently you have local variables inside the constructor, change it to be like:
public class BaseController : Controller
{
PIAEntities db = new PIAEntities();
// Fields added
protected IQueryable<Partenaire> partenaires ;
protected IQueryable<Actualite> sliders ;
public BaseController()
{
sliders = db.Actualite.Where(a => a.Afficher).OrderByDescending(a => a.Date_publication);
partenaires = db.Partenaire.Where(p => p.Afficher);
}
}
Now you will be able to access in other controllers that would inherit from this base controller.
Upvotes: 0
Reputation: 5747
You're getting that error because right now you're only declaring the objects in the scope of the constructor. To fix, declare them outside of the constructor and set the value inside the constructor (not sure if I got the types right, had to deduct them from your example).
public class BaseController : Controller
{
PIAEntities db = new PIAEntities();
IQueryable<Actualite> sliders;
IQueryable<Paternaire> partenaires;
public BaseController()
{
sliders = db.Actualite.Where(a => a.Afficher).OrderByDescending(a => a.Date_publication);
partenaires = db.Partenaire.Where(p => p.Afficher);
}
}
Upvotes: 2