current community

your communities

more stack exchange communities

Stack Exchange
tour help
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.
up vote 1 down vote favorite

I have the following code inside a method:

var addresses = new List<MutateAddressData>();

var yearsDB = (from tr in this.uow.TaxReturns
                where taxReturnIds.Contains(tr.Id)
                select new { Year = tr.TaxYear, TaxReturnTypeId = tr.TaxReturnType.Id }).ToList();

var ibAndVpb = yearsDB.Where(y => y.TaxReturnTypeId == (decimal)TaxReturnType.IB ||
                                  y.TaxReturnTypeId == (decimal)TaxReturnType.VpB);
if (ibAndVpb.Any())
{
    var address = new MutateAddressData
                    {
                    AddressType = AddressType.TaxReturn,
                    CityId = customer.PaCity.Id,
                    CountryId = customer.PaCity.Country.Id,
                    HouseNumber = customer.PaHouseNumber,
                    HouseNumberPostfix = customer.PaHouseNumberPostfix,
                    Street = customer.PaStreet,
                    ZipCode = customer.PaZipCode,
                    Years = string.Join(", ", ibAndVpb.Select(t => t.Year.ToString())),
                    TaxReturnType = (TaxReturnType)ibAndVpb.First().TaxReturnTypeId,
                    };

    addresses.Add(address);
}

var averaging = yearsDB.Except(ibAndVpb);
if (averaging.Any())
{
    var address = new MutateAddressData
                    {
                    AddressType = AddressType.TaxReturn,
                    CityId = customer.PaCity.Id,
                    CountryId = customer.PaCity.Country.Id,
                    HouseNumber = customer.PaHouseNumber,
                    HouseNumberPostfix = customer.PaHouseNumberPostfix,
                    Street = customer.PaStreet,
                    ZipCode = customer.PaZipCode,
                    Years = string.Join(", ", averaging.Select(t => t.Year.ToString())),
                    TaxReturnType = (TaxReturnType)averaging.First().TaxReturnTypeId,
                    };

    addresses.Add(address);
}

return addresses.ToArray();

I want to create max two different objects here. One object for types that er Ib or Vpb and the other object is for all other types.

My senses tell me that I can achieve the same more easily, but I can't figure out how. What do you think?

share|improve this question
    
Can you explain the intent behind averaging.First()? Why the first and not one of the others? Or are they all equal? –  CodesInChaos Mar 3 at 13:16

1 Answer 1

up vote 3 down vote accepted

Considering that most of the options of MutateAddressData seem to be identical for both cases, I'd extract that into a method. But then you'd need yearsDB to contain items of the type TaxReturn.

So you'd basically end up with something like this:

var yearsDB = this.uow.TaxReturns.Where(x => taxReturnIds.Contains(x.Id)).ToList();

var ibAndVpb = yearsDB.Where(x => (TaxReturnType)x.TaxReturnType.Id == TaxReturnType.IB ||
                                  (TaxReturnType)x.TaxReturnType.Id == TaxReturnType.VpB);
if (ibAndVpb.Any())
{
    addresses.Add(CreateMutateAddressData( customer,  ibAndVpb));
}

var averaging = yearsDB.Except(ibAndVpb);
if (averaging.Any())
{
    addresses.Add(CreateMutateAddressData( customer,  averaging));
}

return addresses.ToArray();

This is the method that's called:

private MutateAddressData CreateMutateAddressData(Customer customer, List<TaxReturn> taxReturns)
{
    return new MutateAddressData
                    {
                    AddressType = AddressType.TaxReturn,
                    CityId = customer.PaCity.Id,
                    CountryId = customer.PaCity.Country.Id,
                    HouseNumber = customer.PaHouseNumber,
                    HouseNumberPostfix = customer.PaHouseNumberPostfix,
                    Street = customer.PaStreet,
                    ZipCode = customer.PaZipCode,
                    Years = string.Join(", ", taxReturns.Select(t => t.Year.ToString())),
                    TaxReturnType = (TaxReturnType)taxReturns.First().TaxReturnType.Id,
                    };
}
share|improve this answer

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.