Why my Generic List is always null?

Question

I have class like this:

public class Lugar
{
    [Key]
     public int LugarId { get; set; }


    public List<Review> Reviews { get; set; }
    public int SumReviews { get; set; }

    public double AverageReviews { get {

        if (Reviews == null)
            return 0;

        else if (Reviews.Count == 0)
            return 0;

        else  
        return (double)SumReviews/(Reviews.Count); } }


}

And in my controller I have this:

 [HttpPost, Authorize]
    public ActionResult WriteReview(int id, FormCollection formCollection)
    {

        Lugar lugar = db.Lugares.Find(id);
                    Review review=new Review();
        review.User = User.Identity.Name;
        review.Rating=Convert.ToInt32(formCollection["Rating"]);
        review.Texto = formCollection["Review"];
        if (lugar != null)
        {
            if(  lugar.Reviews==null)
            lugar.Reviews=new List<Review>();

            lugar.Reviews.Add(review);
            lugar.SumReviews += review.Rating;
            db.SaveChanges();

        }

        else
            return RedirectToAction("Index");



        return RedirectToAction("Index");
    }


}

The problem is in the line:

if( lugar.Reviews==null) lugar.Reviews=new List();

Everytime I execute I am getting ( lugar.Reviews==null) as true.....

Even if I already added a Review for that place, the if statement returns true.....

Creating the list manually with `new` is fine in this case and better than marking the list as `virtual` (for lazy loading) which will only cause an unnessecary database query. You don't need to load the whole list from the DB in order to add a new child. — Slauma, Mar 30 '13 at 00:49
If I create a new List everytime I loose my data,I have just tested — Ze Carioca Silva, Mar 30 '13 at 15:31
What do you mean with "*loose my data*"? Do you mean that the reviews that are already stored in the DB get deleted? That cannot happen with the code in your question... — Slauma, Mar 30 '13 at 15:39

score 2 · Accepted Answer · edited Mar 30 '13 at 00:37

2

Try using the 'virtual' keyword where you declare your List and see if you have any more luck.

edited Mar 30 '13 at 00:37

Daniel Mann

57,011
13
100
120

answered Mar 30 '13 at 00:15

Rob G

3,496
1
20
29

I can't find anything about a virtual operator, do you mean the virtual modifier? How would you use it in this case, and why? – Guffa Mar 30 '13 at 00:32
Now I understand why the virtual is necessary.... Its because if I dont use the virtual keyword, I must use Eager Loading or Explicit loading and I was not doing it.... I was not loading my List.... – Ze Carioca Silva Mar 30 '13 at 00:40
The virtual is necessary so that the EF will override that property in your class. As far as I know it only matters when you are using collections of some sort. – Rob G Mar 30 '13 at 00:56

score 0 · Answer 2 · edited May 23 '17 at 10:25

0

You might want to introduce a constructor in your Lugar class and instantiate the list there. Something like this:

public void Lugar()
{
   Reviews = new List<Review>();
}

Hope this helps. of not please let me know.

P.S. another thing that is not related to your specific question, but certainly an improvement is to use view model rather than FormCollection.

It will simplify your life in a major way. For example of how to use it please take a look at this successfully answered question: What is ViewModel in MVC?

edited May 23 '17 at 10:25

Community

1
1

answered Mar 30 '13 at 00:07

Display Name

4,672
1
33
43

Still not working.... Look at the line: else if (Reviews.Count == 0) return 0; I always get Reviews.Count=0 even if I already added a review ... – Ze Carioca Silva Mar 30 '13 at 00:15
I answered purely based on your initial question and had no way to know that was the case? Anyway, if you say in your comments that the initialization of the list with `new` keyword as I suggested works better than adding a `virtual` keyword, you might want to reconsider the accepted answer ... Thanks. – Display Name Mar 30 '13 at 13:20
that initializationd doesnt help me... I still need to do the loading using lazy loading... or I could use the eager or explicit loading... but that viewModel post is helpfull , thx anyway – Ze Carioca Silva Mar 30 '13 at 16:01
Yes, viewmodel is the way to go, it is SO MUCH easier using view model than the way you described it. I mean you can get away your way but its more like bad practice, although working. Anyway, you solved your problem - glad to hear that. Good luck with your dev. – Display Name Mar 30 '13 at 16:44

score 0 · Answer 3 · answered Mar 30 '13 at 00:20

You have 2 options here, lazy loading (which is enabled by putting virtual on navigation properties) This will pull down your second entitiy when you access the property in C# or eager loading by using a .Include(/*lambda or string property name*/) statement in your query.

Personally i perfer eager loading as you have more control over when entities are loaded

Why my Generic List is always null?

3 Answers3