Online examination portal for learning

Question

I am new to ASP.NET and I just developed a simple online examination portal for learning. I used ADO.NET, MySQL and developed in VS 2010.

I have a login page in which the user can login and register for a new user. After successful login, the user is redirected to the question page and I fetch the first question from the database. I populated the question in the label and options in the radio button list. The user can select one option and click the "next" button. In the click event of the "next" button I calculate the marks. I store all values in the session only. When the user click next of last question that is 4 user is redirected to the results page and prints the marks.

 public partial class Questions : System.Web.UI.Page
{
    protected void Page_Load(object sender, EventArgs e)
    {
        Response.Cache.SetCacheability(HttpCacheability.NoCache);
        Response.Cache.SetExpires(DateTime.Now.AddSeconds(-1));
        Response.Cache.SetNoStore();
        if (!IsPostBack)
        {
            renderQuestions(1);
            Session["buttonIndex"] = 1;
            Session["Marks"] = 0;
        }

    }
    public void renderQuestions(int index)
    {

        MySqlConnection con = null;
        string conString = ConfigurationManager.ConnectionStrings["conString"].ConnectionString;
        string qry = "SELECT * FROM QUESTIONS WHERE QUESTION_ID="+index+"";
        try
        {
            using (con = new MySqlConnection(conString))
            {
                con.Open();
                using (MySqlCommand cmd = new MySqlCommand(qry, con))
                {
                    using (MySqlDataAdapter ada = new MySqlDataAdapter(cmd))
                    {
                        DataTable dt = new DataTable();
                        ada.Fill(dt);
                        if (dt.Rows.Count > 0)
                        {
                                clsQuestion ques = new clsQuestion();
                                ques.QuestionId = Convert.ToInt32(dt.Rows[0][0]);
                                ques.Question = Convert.ToString(dt.Rows[0][1]);
                                ques.Option1 = Convert.ToString(dt.Rows[0][2]);
                                ques.Option2 = Convert.ToString(dt.Rows[0][3]);
                                ques.Option3 = Convert.ToString(dt.Rows[0][4]);
                                ques.Option4 = Convert.ToString(dt.Rows[0][5]);
                                ques.Answer = Convert.ToInt32(dt.Rows[0][6]);
                                renderQuesAndAnswers(ques);
                        }
                    }
                }
            }
        }
        catch (Exception ex)
        {
            throw ex;
        }
        finally
        {
            con.Close();
        }
    }
    public void renderQuesAndAnswers(clsQuestion quest)
    {


            lblQuestion.Text = quest.Question;
            RadioButtonList1.Items.Clear();
            RadioButtonList1.Items.Add(quest.Option1);
            RadioButtonList1.Items.Add(quest.Option2);
            RadioButtonList1.Items.Add(quest.Option3);
            RadioButtonList1.Items.Add(quest.Option4);               
            Session["QuestionNumber"] = quest.QuestionId ;
            Session["Answer"] = quest.Answer;
    }
    public   class clsQuestion
    {
        private int questionId;
        private string question;
        private string  option1;
        private string  option2;
        private string  option3;
        private string  option4;
        private int answer;
        public int QuestionId
        {
            get { return questionId; }
            set { questionId = value; }
        }
        public string Question
        {
            get { return question; }
            set { question = value; }
        }
        public string  Option1
        {
            get { return option1; }
            set { option1 = value; }
        }
        public string  Option2
        {
            get { return option2; }
            set { option2 = value; }
        }
        public string  Option3
        {
            get { return option3; }
            set { option3 = value; }
        }
        public string  Option4
        {
            get { return option4; }
            set { option4 = value; }
        }
        public int Answer
        {
            get { return answer; }
            set { answer = value; }
        }


    }

    protected void option1_CheckedChanged(object sender, EventArgs e)
    {
        if (Convert.ToInt32 (Session["Answer"]) == 1)
        {
           int marks=Convert.ToInt32 (Session["Marks"]);
           marks++;
           Session["Marks"] = marks;
        }

    }

    protected void option2_CheckedChanged(object sender, EventArgs e)
    {
        if (Convert.ToInt32(Session["Answer"]) == 2)
        {

            int marks = Convert.ToInt32(Session["Marks"]);
            marks++;
            Session["Marks"] = marks;
        }
    }

    protected void option3_CheckedChanged(object sender, EventArgs e)
    {
        if (Convert.ToInt32(Session["Answer"]) == 3)
        {
            int marks = Convert.ToInt32(Session["Marks"]);
            marks++;
            Session["Marks"] = marks;
        }
    }

    protected void option4_CheckedChanged(object sender, EventArgs e)
    {
        if (Convert.ToInt32(Session["Answer"]) == 4)
        {

        }
    }

    protected void btnNext_Click(object sender, EventArgs e)
    {



    }

    protected void btnNext_Click1(object sender, EventArgs e)
    {
        int buton = Convert.ToInt32(Session["buttonIndex"]);
        if (buton < 5)
        {
            if (RadioButtonList1.SelectedIndex + 1 == Convert.ToInt32(Session["Answer"]))
            {
                int marks = Convert.ToInt32(Session["Marks"]);
                marks++;
                Session["Marks"] = marks;
            }

            Session["buttonIndex"] = Convert.ToInt32(Session["buttonIndex"]) + 1;
            renderQuestions(Convert.ToInt32(Session["buttonIndex"]));
            if (buton == 4)
            {
                Server.Transfer("Results.aspx");
                Session.RemoveAll();
            }
        }

    }

}
}

HTML

<form id="form1" runat="server">
<div>
<h3>Please choose the right answer</h3>
</div>

<table class="style1">
    <tr>
        <td class="style3">
            <asp:Panel ID="Panel1" runat="server">
                <asp:Label ID="lblQuestion" runat="server" Text=""></asp:Label>
            </asp:Panel>
            &nbsp;</td>
        <td class="style4">
            </td>
    </tr>
    <tr>
        <td class="style2">
            Answers:</td>
        <td>
            &nbsp;</td>
    </tr>
    <tr>
        <td class="style2">
            <asp:Panel ID="Panel2" runat="server">
                <asp:RadioButtonList ID="RadioButtonList1" runat="server">
                </asp:RadioButtonList>
                <asp:RadioButton ID="option1" runat="server" Checked="false" AutoPostBack="True" 
                    GroupName="Option" oncheckedchanged="option1_CheckedChanged" />
                <asp:RadioButton ID="option2" runat="server" Checked="false" AutoPostBack="True" 
                    GroupName="Option" oncheckedchanged="option2_CheckedChanged" />
                <asp:RadioButton ID="option3" runat="server" Checked="false" AutoPostBack="True" 
                    GroupName="Option" oncheckedchanged="option3_CheckedChanged" />
                <asp:RadioButton ID="option4" runat="server" Checked="false" AutoPostBack="True" 
                    GroupName="Option" oncheckedchanged="option4_CheckedChanged" />
            </asp:Panel>
        </td>
        <td>
            &nbsp;</td>
    </tr>
    <tr>
        <td class="style2">
            &nbsp;</td>
        <td>
            &nbsp;</td>
    </tr>
    <tr>
        <td class="style2">
            <asp:Button ID="btnNext" runat="server" onclick="btnNext_Click1" Text="Next" />
        </td>
        <td>
            &nbsp;</td>
    </tr>
</table>

</form>

I got the result perfectly and no issues yet. But I want to make sure I am doing it the best way, if my code meets the standards, and if there are any security issues.

Answer 1

Overall, I think this code is a great first try. That said, it has a lot of room for improvement in terms of simplification and improving readability.

As an aside, I'm not sure if you have a strong motivation for using Visual Studio 2010, but if you don't, I would recommend considering using a more up to date version. Visual Studio 2017 was recently released.

Naming Conventions

The naming of the methods should be PascalCase, and should not be abbreviated. For example, instead of renderQuesAndAnswers, use RenderQuestionAndAnswers. This improves readability and is consistent with C# conventions.

You also use Hungarian notation for the question class. I would also denote that it is a model. Instead of clsQuestion, it should simply be QuestionModel.

See https://msdn.microsoft.com/en-us/library/ms229045(v=vs.110).aspx and https://msdn.microsoft.com/en-us/library/ms229043(v=vs.110).aspx for reference. There are some great points in that documentation.

Use C# Auto-Implemented properties

The Question class can be greatly simplified using C# auto-implemented properties. Use public string Property { get; set; } instead of creating you own backing fields. See https://msdn.microsoft.com/en-us/library/bb384054.aspx for reference. I would also rename the Question property to QuestionText to avoid confusion.

With this, the Question class could be extremely simple:

public class QuestionModel {
    public int QuestionId { get; set; }
    public string QuestionText { get; set; }
    public string Option1 { get; set; }
    public string Option2 { get; set; }
    public string Option3 { get; set; }
    public string Option4 { get; set; }
    public int Answer { get; set; }
}

catch and throw

catch (Exception ex) {
    throw ex;
}

This is unnecessary. Without it, it will propagate the exception up, the same as manually rethrowing the exception.

Remove Unneeded Controls

You seem to have both a radio button list and 4 radio buttons. We really only need the radio button list. There are also a few panels that aren't needed.

Use ViewState Instead Of Session

Instead of using the session to store data between questions, we can use the ViewState. The ViewState doesn't persist the data on the server, but rather stores it on the client side. Since we don't need to make use of the internal state data (such as the current question), we can store it in the ViewState. Using ViewState also allows a user to have multiple question sessions going on at a time in separate windows, whereas the session data would otherwise be mixed between them.

We can also use properties to simplify this access. Instead of using strings to access the variables, we can implement the access in the property, which will prevent us from accidentally using the wrong key. For example:

private int QuestionNumber {
    get {
        return (int)ViewState["QuestionNumber"];
    }
    set {
        ViewState["QuestionNumber"] = value;
    }
}

We can also store the entire current question in the ViewState to avoid having to store individual properties of it, such as the answer index. We can do this by adding the [Serializable] attribute to the QuestionModel class. Note that if we do use the ViewState, we should also encrypt it to avoid disclosing the answers and other internal information. To do this, we can add ViewStateEncryptionMode="Always" to the Page directive on the .aspx file.

Also note that we'll need to find a way to provide access to the correct count to Results.aspx. For simplicity, we can use session data for this specific case:

Session["CorrectCount"] = CorrectCount.ToString();
Server.Transfer("Results.aspx");
Session.Contents.Remove("CorrectCount");

Ideally, it would be great to have a result information class that we could serialize and pass through, which might include the percentage, which answers were right/wrong, etc. Note: It really shouldn't be using the session at all here (see reasoning above), but the architecture of transferring the data is out of the scope of this question, so this is here for a proof of concept.

Separate Data Access From Rendering

Currently renderQuestions both handles data access as well as calling the rendering method. I would extract the data access to it's own method, something like:

private QuestionModel GetQuestion(int index) {
    // Data Access Logic
}

This allows us to have a much cleaner render logic:

private void RenderQuestion() {
    CurrentQuestion = GetQuestion(QuestionNumber);

    lblQuestion.Text = CurrentQuestion.QuestionText;

    rblOptions.Items.Clear();
    rblOptions.Items.Add(CurrentQuestion.Option1);
    rblOptions.Items.Add(CurrentQuestion.Option2);
    rblOptions.Items.Add(CurrentQuestion.Option3);
    rblOptions.Items.Add(CurrentQuestion.Option4);
}

Only Handle Answer Checking on Next Click

There's no good reason to check anything about the selected option when it's checked; we really only care when the user clicks next. This greatly simplifies the logic, as we can remove all of those methods, and implement all of the logic in the next button click method. For example:

protected void btnNext_Click(object sender, EventArgs e)
{
    if (rblOptions.SelectedIndex + 1 == CurrentQuestion.Answer) {
        CorrectCount++;
    }

    if (QuestionNumber == 4) {
        Server.Transfer("Results.aspx");
    }
    else {
        QuestionNumber++;
        RenderQuestion();
    }
}

Putting It All Together

With all of this, we end up with the following. Notice that we've reduced the size of the class by almost half and made it a lot more readable.

CodeBehind

using System;
using System.Web;
using System.Data;
using System.Configuration;
using MySql.Data.MySqlClient;

namespace questions
{
    public partial class Question : System.Web.UI.Page
    {
        [Serializable]
        protected class QuestionModel
        {
            public int QuestionId { get; set; }
            public string QuestionText { get; set; }
            public string Option1 { get; set; }
            public string Option2 { get; set; }
            public string Option3 { get; set; }
            public string Option4 { get; set; }
            public int Answer { get; set; }
        }

        private int QuestionNumber { get { return (int)ViewState["QuestionNumber"]; } set { ViewState["QuestionNumber"] = value; } }
        private int CorrectCount { get { return (int)ViewState["CorrectCount"]; } set { ViewState["CorrectCount"] = value; } }
        private QuestionModel CurrentQuestion { get { return ViewState["CurrentQuestion"] as QuestionModel; } set { ViewState["CurrentQuestion"] = value; } }

        protected void Page_Load(object sender, EventArgs e) {
            Response.Cache.SetCacheability(HttpCacheability.NoCache);
            Response.Cache.SetExpires(DateTime.Now.AddSeconds(-1));
            Response.Cache.SetNoStore();

            if (!IsPostBack) {
                QuestionNumber = 1;
                CorrectCount = 0;
                RenderQuestion();
            }
        }

        private QuestionModel GetQuestion(int index) {
            MySqlConnection connection = null;
            var connectionString = ConfigurationManager.ConnectionStrings["conString"].ConnectionString;
            var query = "SELECT * FROM QUESTIONS WHERE QUESTION_ID=" + index;

            try
            {
                using (connection = new MySqlConnection(connectionString))
                {
                    connection.Open();

                    using (var command = new MySqlCommand(query, connection))
                    {
                        using (var adapter = new MySqlDataAdapter(command))
                        {
                            var dataTable = new DataTable();
                            adapter.Fill(dataTable);
                            if (dataTable.Rows.Count > 0)
                            {
                                return new QuestionModel
                                {
                                    QuestionId = Convert.ToInt32(dataTable.Rows[0][0]),
                                    QuestionText = Convert.ToString(dataTable.Rows[0][1]),
                                    Option1 = Convert.ToString(dataTable.Rows[0][2]),
                                    Option2 = Convert.ToString(dataTable.Rows[0][3]),
                                    Option3 = Convert.ToString(dataTable.Rows[0][4]),
                                    Option4 = Convert.ToString(dataTable.Rows[0][5]),
                                    Answer = Convert.ToInt32(dataTable.Rows[0][6])
                                };
                            }
                            else
                            {
                                return null;
                            }
                        }
                    }
                }
            }
            finally
            {
                connection.Close();
            }
        }

        private void RenderQuestion() {
            CurrentQuestion = GetQuestion(QuestionNumber);

            lblQuestion.Text = CurrentQuestion.QuestionText;

            rblOptions.Items.Clear();
            rblOptions.Items.Add(CurrentQuestion.Option1);
            rblOptions.Items.Add(CurrentQuestion.Option2);
            rblOptions.Items.Add(CurrentQuestion.Option3);
            rblOptions.Items.Add(CurrentQuestion.Option4);
        }


        protected void btnNext_Click(object sender, EventArgs e)
        {
            if (rblOptions.SelectedIndex + 1 == CurrentQuestion.Answer) {
                CorrectCount++;
            }

            if (QuestionNumber == 4) {
                Session["CorrectCount"] = CorrectCount.ToString();
                Server.Transfer("Results.aspx");
                Session.Contents.Remove("CorrectCount");
            }
            else {
                QuestionNumber++;
                RenderQuestion();
            }
        }
    }
}

HTML

<form id="form1" runat="server">
    <div>
        <h3>Please choose the right answer</h3>
    </div>
    <table class="style1">
        <tr>
            <td class="style3">
                <asp:Label ID="lblQuestion" runat="server" Text=""></asp:Label>
            </td>
            <td class="style4">&nbsp;</td>
        </tr>
        <tr>
            <td class="style2">Answers:</td>
            <td>&nbsp;</td>
        </tr>
        <tr>
            <td class="style2">
                <asp:RadioButtonList ID="rblOptions" runat="server"></asp:RadioButtonList>
            </td>
            <td>&nbsp;</td>
        </tr>
        <tr>
            <td class="style2">&nbsp;</td>
            <td>&nbsp;</td>
        </tr>
        <tr>
            <td class="style2">
                <asp:Button ID="btnNext" runat="server" onclick="btnNext_Click" Text="Next" />
            </td>
            <td>&nbsp;</td>
        </tr>
    </table>
</form>