10
\$\begingroup\$

These two methods do the exact same thing (or at least they are supposed to!). They both pass a simple test based on a mock context. Neither has been exposed to any integrated testing yet.

  • Would one of the methods be preferred over the other (and why)?
  • Does anyone see any glaring mistakes in the LINQ version (this is my first real attempt at a LINQ query)?

As you can see the first method is using a repository to hit a database. In order to get the data needed, it makes two db calls to two different tables. The second method is an attempt to simplify the code, make it cleaner and more readable. Performance is not a primary concern at this point. It's really more of a learning exercise.

public string GetDealerEmail( string dealerId )
{
    var dealerAddressRepo = StagRepositoryFactory<DealerAddress>.GetRepository( _spcomContext );
    List<DealerAddress> dealerAddress = dealerAddressRepo.GetWhere( x => x.DealerId.Equals( dealerId ) ).ToList( );
    string addressId = dealerAddress.FirstOrDefault( item => item.AddressTypeCode.Equals( "dealer", StringComparison.CurrentCultureIgnoreCase ) ).AddressId;
    var addressRepo = StagRepositoryFactory<Address>.GetRepository( _spcomContext );
    Address address = addressRepo.GetSingle( x => x.AddressId.Equals( addressId ) );
    return address.Email;
}



public string GetDealerEmailWithLinq( string dealerId)
{
    ISpcomContext context = _spcomContext as ISpcomContext;
    var query = from dealerAddress in context.DealerAddresses
                where dealerAddress.DealerId.Equals( dealerId )
                where dealerAddress.AddressTypeCode.Equals( "dealer", StringComparison.CurrentCultureIgnoreCase )
                join address in context.Addresses
                    on dealerAddress.AddressId equals address.AddressId
                select address.Email;
    var emailAddress = query.FirstOrDefault( );
    return emailAddress;    
}
\$\endgroup\$
4
  • 1
    \$\begingroup\$ What library are you using in the first version of the method? \$\endgroup\$
    – svick
    Commented Sep 19, 2012 at 20:19
  • \$\begingroup\$ @svick It's using a generic repository for database access via DbContext and EF 4.1 \$\endgroup\$
    – Forty-Two
    Commented Sep 19, 2012 at 21:02
  • \$\begingroup\$ I would go with option 2 encapsulated within a repository method. I personally find it easier to read whilst putting inside the repository you can reuse elsewhere if required. \$\endgroup\$
    – dreza
    Commented Sep 19, 2012 at 21:12
  • 3
    \$\begingroup\$ You may also use Include() as Third option. Check out this blogs.msdn.com/b/adonet/archive/2011/01/31/… context.DealerAddresses.Where(d => d.DealerId.Equals( dealerId ) && d.AddressTypeCode.Equals( "dealer", StringComparison.CurrentCultureIgnoreCase )). Include(d => d.Addresses).Select(d => d.Email).FirstOrDefault(); \$\endgroup\$ Commented Sep 22, 2012 at 5:35

1 Answer 1

3
\$\begingroup\$

I personally like the Second option, like you said it is cleaner, easier to read, and looks fairly simple and straight to the point.

I don't see any obvious mistakes in your code.

I can only suggest that you test it vigorously and perhaps check out the option given by @Jignesh Thakker in the comments.


you could reduce the amount of code by merging a variable to your return statement in the second block of code like this.

return query.FirstOrDefault( );

Instead of

var emailAddress = query.FirstOrDefault( );
return emailAddress;
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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