Can someone give me a quick code review for HTML5 site?

Started by
4 comments, last by Alpha_ProgDes 10 years ago

So basically, 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. That's why I'm using neither. I basically created an empty web site and added the files I needed. No more, no less. So I posted this little HTML5 site on GitHub. Nothing fancy. At all.

However, I did structure the folders and files in a MVC-like way. And also I used razor/cshtml file to mimic code-behind. Completely separating the HTML from the server-side code. Also, the JQuery and Javascript is in its own file as well, again, completely separated from the HTML. I have one line of CSS in the HTML.

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?

The link.

Note: If anyone wants to test it, all the files are on GitHub. Even the scripts to create the database. All anyone has to do is: create an empty web site in WebMatrix or Visual Studio. Download the project from GitHub and copy them straight into the website. Configuration files are already setup. All that has to be done is replace DB\DBExpress with the name of the actual database.

Beginner in Game Development?  Read here. And read here.

 

Advertisement

:(

No one?

Beginner in Game Development?  Read here. And read here.

 

Mainpage.cs


    using System;
    using System.Collections.Generic;
    using System.Linq;
    using System.Web;
    using System.Data;
    using System.Data.SqlClient;
    using System.Configuration;
    
    /// <summary>
    /// Summary description for Mainpage
    /// </summary>
    ///
    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;
    
                SqlParameter sp;
                if (DateMade.HasValue)
                    sp = new SqlParameter("@p2", DateMade.Value);
                else
                    sp = new SqlParameter("@p2", DBNull.Value);
    
                SqlParameter[] sps = new SqlParameter[] {new SqlParameter("@p1", LineDesc),
                                                         sp,
                                                         new SqlParameter("@p3", Options1),
                                                         new SqlParameter("@p4", Options2)};
                sqlCmd.Parameters.AddRange(sps);
                
                sqlConn.Open();
                sqlCmd.ExecuteNonQuery();
                sqlConn.Close();
            }
        }
    }


Default.cs (ViewModel)


    using System;
    using System.Collections.Generic;
    using System.Linq;
    using System.Web;
    
    /// <summary>
    /// Summary description for Default
    /// </summary>
    ///
    namespace ViewModels
    {
        public class Default
        {
    
            public Default()
            {
                //
                // TODO: Add constructor logic here
                //
            }
    
            public Default(string desc, string sop1, string sop2, string sop3)
            {
                Description = desc;
                SelectedOp1 = sop1;
                SelectedOp2 = sop2;
                SelectedOp3 = sop3;
            }
    
            public string Description { get; set; }
            public string SelectedOp1 { get; set; }
            public string SelectedOp2 { get; set; }
            public string SelectedOp3 { get; set; }
            public List<string> Option1 { get; set; }
            public List<string> Option2 { get; set; }
            public List<string> Option3 { get; set; }
    
            public void Save()
            {
                Models.Mainpage mp = new Models.Mainpage(0, Description, new DateTime?(DateTime.Now), SelectedOp1, SelectedOp2);
                mp.Save();
            }
        }
    }



Default.cshtml (Controller/Code behind)


    @{
        var linedesc = Request.Form["linedesc"];
        var datemade = DateTime.Now.ToString();
        var food = Request.Form["options1"];
        var drinks = Request.Form["options2"];
        
        string[] data = new String[4] {linedesc, datemade, food, drinks};
        
        // Go back to Default.html and keep values selected, ie. values don't reset
        var blah = "hi";
    
        Response.Cache.SetCacheability(HttpCacheability.NoCache);
            
        if (IsPost)
        {
            ViewModels.Default vm = new ViewModels.Default(linedesc, food, drinks, null);
            vm.Save();
        }
        Json.Write(data, Response.Output);
        
    }


Default.html (View)


   <!DOCTYPE html>
    <html xmlns="http://www.w3.org/1999/xhtml">
    <head>
        <title>Testing Stuff</title>
        <script type="text/javascript" src="/Views/JS/jquery-2.0.3.js"></script>
        <script type="text/javascript" src="/Views/JS/Default.js"></script>
    </head>
    <body>
        <form id="mainpage" method="post" action="../Controllers/Default">
            <div style="clear:both;" id="dbInput">
                <label for="linedesc">Description:</label>&nbsp;
                <input type="text" id="linedesc" name="linedesc" />
    
                <br /><br />
    
                <input id="fruits" name="options1" value="fruits" type="radio" />
                <label for="fruits">Fruits</label>&nbsp;
                <input id="candies" name="options1" value="candies" type="radio" />
                <label for="candies">Candies</label>&nbsp;
                <input id="snacks" name="options1" value="snacks" type="radio" />
                <label for="snacks">Snacks</label>
    
                <br /><br />
    
                <label for="options2">Choose beverage:</label>&nbsp;
                 <select id="options2" name="options2">
                    <option value="Coca-Cola">Coca-Cola</option>
                    <option value="Sprite">Sprite</option>
                    <option value="Root Beer">Root Beer</option>
                    <option value="Orange Juice">Orange Juice</option>
                </select>
    
                <br /><br />
    
                <label for="options3">Sample:</label>&nbsp;
                <select id="options3" name="options3">
    
                </select>
    
                <input type="submit" id="submit1" name="submit1" value="Submit" />
            </div>
        </form>
    </body>
    </html>

Default.js (JavaScript)


   $(document).ready(function () {
        //$('#submitText').click(function () {
        // $('#txtHolder').html('<span>I am like code behind in txtHolder.</span>');
        // $('#results').html('<span>Results are here today.</span>');
        //});
    
        $('#mainpage').submit(function (e) {
            e.preventDefault();
            alert("hi");
            var formData = $(this).serialize();
            var frm = $(e.target);
            $.ajax({
                url: "../../Controllers/Default.cshtml",
                data: formData,
                type: "POST",
                dataType: "json",
                success: function (response) {
                    alert(response);
                    var options3 = $("#options3");
                    options3.empty();
                    for (var i = 0; i < response.length; i++)
                    {
                        options3.append(
                            $("<option></option>").text(response[i]).val(response[i])
                        );
                    }
                    // Adds data to dropdown
                },
                error: function () {
                    alert("Sorry, there seems to be a problem contacting the server.");
                }
            });
        });
    });

Beginner in Game Development?  Read here. And read here.

 

I don't have any experience with C# or CSHTML, so I won't try to provide any feedback about that.

Also I'm not sure what kind of level of feedback you're looking for with the other code, but I will provide some quick, subjective feedback on the HTML and JS just in case it's useful for you...

HTML:

<br>'s and &nbsp's are kind of old-school hacks and are thought of as ugly these days, because you should be putting things that control layout in the CSS, not the HTML.

Inline styles are also frowned upon - it's neater to put your styles in a separate CSS file.

Script tags are best moved to the bottom of the HTML page (just before </body>) so they don't block page rendering. (Or you could try using the 'async' or 'defer' attribute but you may want to read up and check on the browser support).

Is this just intended for desktop browsers? If you would like to do things nicely for mobile too then you will probably want to add a viewport meta tag, but avoid "user-scalable=no" and "maximum-scale" if possible because that will most likely cause an accessibility issue.

I would avoid using IDs like "options2" and "options3" because it will be harder for other developers to know what they're referring to and it implies an order that might change. Better to use things that describe those options, e.g. "beverageOptions".

JavaScript:

It's generally thought better to declare your initial function (e.g. 'init') and then just reference it inside the JQuery document.ready function, rather than sticking everything inside. That'd help if you decide to use another way of determining whether it's ready. Also there's a shortcut for document.ready you may like to use: $(yourFunctionName);

Also similarly for callbacks, it can make it easier to read if, instead of using an anonymous function in place, you declare the method separately and then just reference it. E.g. instead of:


success: function (response) {
 ...
}
You can do:

function onPostSuccess(response) {
 ...
}


success: onPostSuccess
Just helps a bit to avoid ending up with massive nested code blocks.
Anyway, that kind of thing is more for if you end up growing a bigger codebase out from this starting point, and not really something you need to worry about just for a simple test site. So not sure if it's the kind of thing you're looking for or not.

You didn't include the .sln or the .csproj files as part of your commit - so it's impossible for anyone reviewing this to actually be able to compile and run your project. Without that, we're limited to just looking at the code and guessing at what it's supposed to do and how it's all supposed to fit together. I suggest you grab the .gitignore file from here:

https://github.com/github/gitignore/blob/master/VisualStudio.gitignore

Save it to your project root, and then commit anything that it doesn't filter out.

Also, are you using ASP.NET MVC proper, or are you using ASP.NET WebForms but with an MVC-like architecture? I can't really tell because you got .a cshtml file with what appears to be Razor syntax, and you're calling it the "controller" (slash "code behind"). Controllers should have a .cs extension (they're classes that inherit from the Controller class, and have methods that return ActionResults), and they shouldn't have Razor in them - so that really looks more like a view. Also, there is no concept of "code behind" in ASP.NET MVC. So I'm guessing you might be using WebForms? But then you would have .aspx files, not .cshtml. So in short, I'm a bit confused - you seem to be mixing or misusing frameworks here. Or maybe it's me who's losing my mind. (Please inform me immediately if that appears to be the case.) If you're intending to use MVC, then I strongly suggest you work your way through some tutorials first.

It started just as "ASP.NET Empty Web Site". No Webforms, no MVC. I attempted to put the files and the code in a MVC like organization. But as I noted in the OP I didn't want the magic. Everything is hand coded.

You should be able to create an empty web site and just copy the files as is in there.

Beginner in Game Development?  Read here. And read here.

 

This topic is closed to new replies.

Advertisement