patterncsharpMinor

Is my web site structured correctly?

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

correctlystructuredsiteweb

Problem

I'm trying to get a better handle of code organization and how HTML5 (really HTML) stack works in general. I've worked mostly with ASP.NET Webforms and done some MVC as well. Let me say now, "I hate magic". I hate figuring out the magic. I just want it to work in the manner that it's supposed to. So I posted this little HTML5 site on GitHub. Nothing fancy. At all. The structure is very MVC like and I do as much separation of files and code-behind as possible.

All I'm using is: HTML, JavaScript, jQuery, no CSS, and CSHTML (as codebehind).

So in short, is this good? Is this bad? What could be better?

Code on Github

Mainpage.cs (Model)

```
using System;
using System.Collections.Generic;
using System.Linq;
using System.Web;
using System.Data;
using System.Data.SqlClient;
using System.Configuration;

///
/// Summary description for Mainpage
///
///
namespace Models
{
public class Mainpage
{
private SqlConnection sqlConn;
private string connStr;

public Mainpage()
{
//
// TODO: Add constructor logic here
//
}

private void sqlInit()
{
connStr = ConfigurationManager.ConnectionStrings["TestInput"].ConnectionString;
sqlConn = new SqlConnection(connStr);
}

public Mainpage(int id, string ld, DateTime? dm, string op1, string op2)
{
ID = id;
LineDesc = ld;
DateMade = dm;
Options1 = op1;
Options2 = op2;
}

public int ID { get; set; }
public string LineDesc { get; set; }
public DateTime? DateMade { get; set; }
public string Options1 { get; set; }
public string Options2 { get; set; }

public void Save()
{
sqlInit();
string insertStr = "usr_InsertValues";
SqlCommand sqlCmd = new SqlCommand(insertStr, sqlConn);
sqlCmd.CommandType = CommandType.StoredProcedure;

Solution

On the HTML side, don't use

to force the content to a new line. Place contents in `

 instead.


  Description: 
  ...

  
  ...

  Choose beverage: 
  ...

  Sample: 
  ...


On the JS side of things, load scripts last to avoid blocking the UI. It's better if the user can see the page first while scripts load, rather than a blank page because scripts load. Place them before

` preferrably.

And some script improvements:

// A function to a jQuery function is shorthand for $(document).ready(fn);
$(function () {

  // I highly guess this is a static container. I'd cache it
  // in a variable to avoid jQuery fetching it again every submit.
  var options3 = $("#options3");

  //Commented out code is useless code. Get rid of it.

  $('#mainpage').submit(function (e) {
    e.preventDefault();

    // And unless you have reason for the alert, don't use it.
    // In debugging, console.log() is better, and breakpoints are even better than both

    //e.target and "this" in this handler is the same
    var form = $(this);
    var formData = form.serialize();

    // You can use the form action attribute for the url. Don't hardcode. 
    // If it's different, you can store it in a data-* variable
    // Use data-* variables for dynamic stuff generated on page.
    // Helps you get along with backend devs who generate the HTML but hate writing JS (or mixing code with it);
    var url = form.attr('action'); //form.data('action') if it was in data-action="URL" in the HTML

    // Shorthand AJAX post. Nulled the third since we use promises.
    $.post(url, formData, null, 'json')
      .done(function (response) {
        options3.empty();

        for (var i = 0; i ').text(response[i]).val(response[i]).appendTo(option3);
        }
        // Adds data to dropdown
      }).fail(function () {
        // In this case, it's a warning. I'll let the alert through.
        // There are things called "modals" too, much prettier than alerts. Just sayin' :D
        alert("Sorry, there seems to be a problem contacting the server.");
      });
  });
});

Then I'm no expert in C# and the backing data store, but it's better if on the values of your choices, you used the ID's of the choices rather than the text. Scenario: What if someone modified the data to have 2 or more "Coca-Cola" entries. How would you know which is which?

But, if you had a unique id for each item, then you'd know which one regardless if you named them the same. In the real world, that's how you differentiate 2 or more people of the same name on Facebook (or any site for that matter) - the user id.

// In the HTML

  Coke
  Pepsi
  Coke
  Dr. Pepper

// In your data storage, something like:
[
  {"id":1,"text":"Coke"},        <-- This is a different coke
  {"id":2,"text":"Pepsi"},
  {"id":3,"text":"Coke"},        <-- This is also a different coke
  {"id":4,"text":"Dr. Pepper"}
]

I'll leave the C# to the other guys here.

Code Snippets

<div>
  <label for="linedesc">Description:</label>&nbsp;
  ...
</div>
<div>
  <input id="fruits" name="options1" value="fruits" type="radio" />
  ...
</div>
<div>
  <label for="options2">Choose beverage:</label>&nbsp;
  ...
</div>
<div>
  <label for="options3">Sample:</label>&nbsp;
  ...
</div>

<script type="text/javascript" src="/Views/JS/jquery-2.0.3.js"></script>
    <script type="text/javascript" src="/Views/JS/Default.js"></script>
</body>

// A function to a jQuery function is shorthand for $(document).ready(fn);
$(function () {

  // I highly guess this is a static container. I'd cache it
  // in a variable to avoid jQuery fetching it again every submit.
  var options3 = $("#options3");

  //Commented out code is useless code. Get rid of it.

  $('#mainpage').submit(function (e) {
    e.preventDefault();

    // And unless you have reason for the alert, don't use it.
    // In debugging, console.log() is better, and breakpoints are even better than both

    //e.target and "this" in this handler is the same
    var form = $(this);
    var formData = form.serialize();

    // You can use the form action attribute for the url. Don't hardcode. 
    // If it's different, you can store it in a data-* variable
    // Use data-* variables for dynamic stuff generated on page.
    // Helps you get along with backend devs who generate the HTML but hate writing JS (or mixing code with it);
    var url = form.attr('action'); //form.data('action') if it was in data-action="URL" in the HTML

    // Shorthand AJAX post. Nulled the third since we use promises.
    $.post(url, formData, null, 'json')
      .done(function (response) {
        options3.empty();

        for (var i = 0; i < response.length; i++) {
          // jQuery also accepts self-closing tags to create elements.
          // Also, there's an appendTo() function, which looks much cleaner in this case
          $('<option/>').text(response[i]).val(response[i]).appendTo(option3);
        }
        // Adds data to dropdown
      }).fail(function () {
        // In this case, it's a warning. I'll let the alert through.
        // There are things called "modals" too, much prettier than alerts. Just sayin' :D
        alert("Sorry, there seems to be a problem contacting the server.");
      });
  });
});

// In the HTML
<select>
  <option value="1">Coke</option>
  <option value="2">Pepsi</option>
  <option value="3">Coke</option>
  <option value="4">Dr. Pepper</option>
</select>

// In your data storage, something like:
[
  {"id":1,"text":"Coke"},        <-- This is a different coke
  {"id":2,"text":"Pepsi"},
  {"id":3,"text":"Coke"},        <-- This is also a different coke
  {"id":4,"text":"Dr. Pepper"}
]

Context

StackExchange Code Review Q#47652, answer score: 5

Revisions (0)

No revisions yet.