Ancient ASP.NET structure, tell me how to do it right [closed]

Question

Closed. This question is opinion-based. It is not currently accepting answers.

---

Want to improve this question? Update the question so it can be answered with facts and citations by editing this post.

Closed 7 years ago.

Improve this question

I'm pretty new to ASP.NET and I think im not using it they way it's meant to be used with all the features packed into the latest .NET framework. I'm currently using .NET framework 4,0. There are some error in the code, don't mind them mind they way I seem to be using ancient techniques.

I have structured everything like this.

I file called webservice.cs, that file is packed with webmethods like this:

[WebMethod]
public string laggtillprodukt(string pro1, int pro2)
{
    int sqlstatus;
    string sqlinsertstringfull = "INSERT INTO t_produkter (produkt_namn) VALUES ('" + pro1 + "');" +
                                 "SELECT produkt_id FROM t_produkter WHERE (produkt_id = SCOPE_IDENTITY()); " +
                                 "INSERT INTO t_produktegenskaper (produkt_id, egenskaps_id) " +
                                 "SELECT SCOPE_IDENTITY(), egen.egenskap_id " +
                                 "FROM t_kopplingmallegenskaper as egen " +
                                 "WHERE egen.mall_id = " + pro2 + ";";

   sqlstatus = executeWriteSqlQuery(sqlinsertstringfull);

   return "These values has been added to the db" + pro1 + " and " + pro2 + " SQL STATUS:" + sqlstatus;
}

In my code behind i do this to call the correct function(the one below has nothing to do with the webmethod before it was just to illustrate one of many SQL QUERIES.

 protected void laggtillnymallbutton_Click(object sender, EventArgs e)
{
    WebService globalwebservice = new WebService();

    if (string.IsNullOrWhiteSpace(laggtillnymall.Text))
    {
        Label1.Text = "String cannot be empty or just whitespaces!";
    }
    else
    {
        globalwebservice.laggtillmall(laggtillnymall.Text.Trim());
        Label1.Text = "Template added";
    }

Can't I be doing this in a more effiecient way. I have constructed a general method that all webmethods either use to insertdata or to read data, saved me some code, but I've seen things like LINQ. That has far less code than I have. Please aid me or point me in a towards a not so ancient way of coding ;)

Answer 1

What I think you're asking is a better way to access your database. You should look into Linq-To-SQL and Entity Framework for starters. These technologies provide a more modern and easier-to-work with approach in dealing with your data stores.

Answer 2

Well, let me see....

• you fail to decouple business logic from DAL, that's a VERY bad approach. Data access should be in a completely separate layer, it doesn't belong to webservice.

• your code is terribly vulnerable to SQL injection https://www.owasp.org/index.php/SQL_Injection . Instead, you should be using parameterized queries http://www.techrepublic.com/article/shorten-development-time-by-using-parameterized-queries-in-adonet/6093390 or stored procedures, depending on your needs.

• your naming convention isn't great as well... In .NET we usually prefer to use camelCase, not "thisismysuppaduppamethod", and your variable names aren't really clear. Have in mind, that you're not writing the code for the machine, but rather for people to read. Code should be easily readable and anyone who looks at your code must immediately understand, what's its purpose. I highly doubt, that you'll see immediately what does string pro1, int pro2 actually mean, without studying the code deeper, if you ever need to revise your code in the future, let's say in 2-3 years later.

• I'd recommend using English instead of your native language. If you'll ever need help with the code (or introduce a new co-worker to your team), it's usually considered a superior practice, because you won't have to explain "and what does this stand for??".