HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Multiple repository calls or LINQ query

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
callslinqqueryrepositorymultiple

Problem

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.GetRepository( _spcomContext );
    List dealerAddress = dealerAddressRepo.GetWhere( x => x.DealerId.Equals( dealerId ) ).ToList( );
    string addressId = dealerAddress.FirstOrDefault( item => item.AddressTypeCode.Equals( "dealer", StringComparison.CurrentCultureIgnoreCase ) ).AddressId;
    var addressRepo = StagRepositoryFactory.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;    
}

Solution

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;

Code Snippets

return query.FirstOrDefault( );
var emailAddress = query.FirstOrDefault( );
return emailAddress;

Context

StackExchange Code Review Q#15737, answer score: 3

Revisions (0)

No revisions yet.