One should always use command parameters (@name) in SQL statements. That will prevent SQL injection attacks with or without the use of stored procs. I have also found the use of stored procedures for everything to be a waste of time. I create procs for insert, update and delete. Selects I put directly in the dataprovider because there are many of them and they change frequently. There is no real-life performance benefit with stored procedures.  A busy site will have all commands optimized within SQL server anyways (remember to use parameters). And there isn't a security benefit with stored procs either.

But there is one big problem with your code. You don't handle prefixes properly. All tables in a DNN database are sometimes prefixed with a short name for historical reasons. If you don't take this into account your code will fail in some environments. Even though it is no longer recommended to use the prefix feature, it is still supported by the installation wizard and many older DNN sites are using them. 

 Thanks Lars for you reply. Could you correct my code so that it will handle the prefixes because I do not know how to handle it. make it for one function and I will make the rest.

Thanks in advance.