0

I am trying to produce a list of Units that are required to carry out an audit with the date of the audit if its been carried out and if no audit leave if blank.

I have the following in my Get method:

public ActionResult AuditReportList(int stnAssureAuditId)
    {
        //get the data for the list
        var people = new List<People.Models.Person>(Peopledb.People);
        var reports = new List<StnAssureAuditReport>(db.StnAssureAuditReports);
        var units = new List<People.Models.Unit>(Peopledb.Units);
        var auditUnits = new List<StnAssureUnit>(db.StnAssureUnits).Where(x => x.StnAssureAuditId == stnAssureAuditId);

        var auditReportList = from u in auditUnits
                              join r in reports on u.UnitId equals r.UnitId into ur
                              from a in ur.DefaultIfEmpty()
                              select new
                              {
                                  CarriedOut = (a == null ? String.Empty : a.CarriedOut.ToLongDateString()),
                                  StnAssureAuditReportId = (a == null ? 0 : a.StnAssureAuditReportId),
                                  UnitId = a.UnitId,
                                  Complete = (a == null ? false : true),
                                  StnCommId = a.StnCommId,
                                  WatchCommId = a.WatchCommId
                              };

        var auditUnitsList = from u in auditReportList
                             join r in units on u.UnitId equals r.UnitId
                             select new 
                             {
                                 UnitId = u.UnitId,
                                 UnitName = r.UnitName,
                                 CarriedOut = u.CarriedOut,
                                 StnAssureAuditReportId = u.StnAssureAuditReportId,
                                 Complete = u.Complete,
                                 StnCommId = u.StnCommId,
                                 WatchCommId = u.WatchCommId
                             };

        var reportStnComm = (from c in auditUnitsList
                             join d in people on c.StnCommId equals d.PersonId
                             select new ReportStnComm
                             {
                                 StnAssureAuditReportId = c.StnAssureAuditReportId,
                                 StnComm = d.FirstName + " " + d.LastName,
                                 WatchCommId = c.WatchCommId,
                                 UnitName = c.UnitName,
                                 CarriedOut = c.CarriedOut,
                                 UnitId = c.UnitId,
                                 Complete = c.Complete,
                             }).ToList();

        var reportList = (from h in reportStnComm
                          join f in people on h.WatchCommId equals f.PersonId
                          select new StnAssureReportList
                          {
                              CarriedOut = h.CarriedOut,
                              StnAssureAuditReportId = h.StnAssureAuditReportId,
                              StnComm = h.StnComm,
                              UnitName = h.UnitName,
                              WatchComm = f.FirstName + " " + f.LastName,
                              UnitId = h.UnitId,
                              Complete = h.Complete,
                          }).OrderBy(x => x.UnitName).ToList();


        var viewModel = reportList.Select(t => new AuditReportListViewModel
        {
            CarriedOut = t.CarriedOut,
            StnAssureAuditReportId = t.StnAssureAuditReportId,
            StnComm = t.StnComm,
            UnitName = t.UnitName,
            WatchComm = t.WatchComm,
            Complete = t.Complete
        });

        return View("AuditReportList", viewModel);
    }

However when I run it I get

Object reference not set to an instance of an object.

On the auditReportList variable.

Looking at the debug, a does appear to be null after three loops through, i presumed that in that case the DefaultIfEmpty would kick in so that the a == null can be sused in the select but it doesnt seems to be working

Phill Sanders
  • 487
  • 2
  • 10
  • 30

2 Answers2

1

Guarding against null (ternaries a == null ? ... : ...) on some cases while forgetting others is a mistake. You should guard on all cases. Here, it is not done on UnitId, StnCommId and WatchCommId.

Side note: in my team I have asked for banning linq sql like syntax. It looks to me as a 'false good idea'. It was appealing to write sql like compiled queries in .Net, but in practice we found them to be far less readable and understandable than linq lambda based extensions (as Where(a => ...), Select(a => ...), ...).

Frédéric
  • 9,364
  • 3
  • 62
  • 112
  • You say "far less" readable? That's a bit of an exaggeration I think. I've seen many a multiple `.SelectMany(...)` that are killers when expressed as lambdas, but quite neat as a LINQ query. I do think it is a matter choosing the right horse for the race at hand. – Enigmativity Jan 27 '16 at 12:39
  • @Enigmativity Well, as written, I am speaking of the experience in my current team. Our `SelectMany` cases have not yet proven less readable. It may also be a matter of the tech focus of team members (primarily .Net developers, secondary SQL queries writers), which leads us to let SQL syntax in real SQL queries while using code like syntax (lambda) in .Net code. Granted, 'far less' is for the general case an exaggeration. (I tend to think 'far less' due to monster cases I have encountered, which were really easier once rewritten with lambda.) – Frédéric Jan 27 '16 at 13:09
  • @PhillSanders, as highlighted by Enigmativity in [his answer](http://stackoverflow.com/a/35037465/1178314), you may not need to guard on each case if the queries is against linq2entities/linq2sql and the like, which then get translated to a real SQL query. So you may meet some cases where such queries would work with null values though some `null` testing are 'missing'. But as he said, this is clearly not the case of your current trouble. – Frédéric Jan 27 '16 at 13:22
0

Your issue is with the line from a in ur.DefaultIfEmpty(). If ur is empty then a has the value null of type StnAssureAuditReport, so then when the query tries to evaluate u.UnitId you get a NullReferenceException.

The use of from a in ur.DefaultIfEmpty() select { u.UnitId } only works when ur is IQueryable<StnAssureAuditReport> - the translation to SQL automatically handles the null case for you. But when ur is IEnumerable<StnAssureAuditReport> (as is your case) then the u.UnitId get evaluated and the null exception raised.

I've focused on the u.UnitId property, but the same is true for all of the u.* calls.

Enigmativity
  • 113,464
  • 11
  • 89
  • 172