Comments on a very bad idea

Question

I got an really bad idea :)

When exploring extension methods in vb I suddenly thought what about making an extension method on string to execute data access code.

<Extension()> Function Execute(ByVal s As String) As Data.DataTable
  'make code here to access the database ..
  'read connectionstring from the .config file 

End Function

Then in your website you could do something like this..

<%For Each dr In "select * from product".Execute.Rows%>
   some HTML output here..
<%Next%>

I know this is NOT the way to do it, but in really simple applications, or in prototypes it would be really simple and straightforward.

Any comments?

Answer 1

I can't see that it would be simpler than just having it as a Execute method that takes a string as a parameter. This would just be confusing.

Answer 2

I think you've answered your own question - it's not a great idea. Extension methods are useful in many cases for the sake of convenience, but semantic inconsistency/confusion outweights that here.

What you are doing is creating an extension method that pretends to be a function (method) that acts on all strings, but actually is only meaningful in the narrow context of SQL queries on your database. This sort of mismatch of semantic scope suggests an extension method isn't appropiate here, though a static helper method would be perfectly fine.

Answer 3

I think you've answered your own question - it's not a great idea. Extension methods are useful in many cases for the sake of convenience, but semantic inconsistency/confusion outweights that here.

I completely agree with Noldorin but I would like to expand on his answer.

We use mappers in our code base and they all use an IMaper<TInput, TOutput> interface. We also used to have each of our mappers implement a MapAll method for mapping an input IEnumerable to an output IEnumerable and each implementation was the same. The proposal was to create an extension method for the MapAll but the question was what object was it going to be an extension method for:

public IEnumerable<TOutput> MapAll<TInput, TOutput>(
    this IEnumerable<TInput>, IMapper<TInput, TOutput>)

Or

public IEnumerable<TOutput> MapAll<TInput, TOutput>(
    this IMapper<TInput, TOutput>, IEnumerable<TInput>)

Whilst they would both do the same job the first one would be really confusing as the MapAll method is only relevant in certain cases but it will always be present in your intelisense. where as the second approach whilst you may not need the MapAll method all the time it is clear as to what the purpose of the method is.

So to make your proposal work you could create an extension method for the IDbCommand interface which takes in a string instead (Excuse my attempt a VB.Net):

<Extension()> Function Execute(
    ByRef con As IDbConnection, ByVal command as String) As Data.DataTable
    ...
End Function

And use it like so:

<%For Each dr In conn.Execute("select * from product").Rows%>
    some HTML output here..
<%Next%>

I think the method name is unclear and should be something like GetDataTable but you can get an idea of what I mean.

If you want you can see my blog post on the mapper interface and extension method here to give you some context as to what I was on about.