What is your opinion about this?

Question

0.00/5 (No votes)

See more:

this is my first program ( it's boring i think but i have to practice alot ). What is your opinion about this? can make it faster ?

this is my login code :

C#

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Windows.Forms;
using System.Data.SqlClient;


namespace Film
{
    public partial class Login : Form
    {
        string SqlStr;
        SqlCommand SqlCmd;
        SqlDataAdapter SqlDa;
        SqlDataReader SqlDr;

        public Login()
        {
            InitializeComponent();
        }


        private void button2_Click(object sender, EventArgs e)
        {
            this.WindowState = FormWindowState.Minimized;
        }

        private void button3_Click(object sender, EventArgs e)
        {
          

            this.Close();

            
        }

        private void button1_Click(object sender, EventArgs e)
        {
            
             SqlConnection CN = new SqlConnection("Data Source=(local);Initial Catalog=Movies;Integrated Security=True");
            try
            {
                CN.Open();

                SqlStr = "   select LoginName,LoginPass ";
                SqlStr = SqlStr + " from Login";
                SqlStr = SqlStr + " where LoginName='" + TxtUserName.Text + "'  AND LoginPass='"+TxtPassword.Text+"' ";

                SqlCmd = new SqlCommand(SqlStr, CN);
                SqlDr = SqlCmd.ExecuteReader();

                SqlDr.Read();

                if (!SqlDr.HasRows)
                {
                    MessageBox.Show("Wrong username or Password!!");
                }
                else
                {
                    MovieLib frm = new MovieLib();
                    this.Hide();
                    frm.Show();
                  
                }

            }

            finally
            {
                CN.Close();
            }
        }

        private void pictureBox2_Click(object sender, EventArgs e)
        {

        }
            
            
        
        }
    }

and this is form1 :

C#

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Windows.Forms;
using System.Data.SqlClient;

namespace Film
{
    public partial class MovieLib : Form
    {
        string SqlStr;
        SqlCommand SqlCmd;
        SqlDataAdapter SqlDa;
        SqlDataReader SqlDr;

        public MovieLib()
        {
            InitializeComponent();
        }

        public void Clear()
        {
            TxtName.Text = "";
            TxtDirector.Text = "";
            TxtYear.Text = "";
            TxtType.Text = "";
        }

        private static SqlConnection CreateConnection()
        {
            return new SqlConnection("Data Source=(local);Initial Catalog=Movies;Integrated Security=True");
        }

        private void BtnAdd_Click(object sender, EventArgs e)
        {
            using (SqlConnection connection = CreateConnection())
            using (SqlCommand command = new SqlCommand("Insert into Movies([Name], [Director], [Year], [Type]) VALUES (@Name, @Director, @Year, @Type)", connection))
            {
                command.Parameters.AddWithValue("@Name", TxtName.Text);
                command.Parameters.AddWithValue("@Director", TxtDirector.Text);
                command.Parameters.AddWithValue("@Year", TxtYear.Text);
                command.Parameters.AddWithValue("@Type", TxtType.Text);

                try
                {
                    connection.Open();
                    command.ExecuteNonQuery();

                    MessageBox.Show("Inserted successfully.");
                    Clear();
                }
                catch (SqlException ex)
                {
                    MessageBox.Show(ex.Message);
                    // TODO: Log the error somewhere
                }
            }
        }

        private void BtnEdit_Click(object sender, EventArgs e)
        {
            using (SqlConnection connection = CreateConnection())
            using (SqlCommand command = new SqlCommand("UPDATE Movies SET Name = @Name, Director = @Director, Year = @Year, Type = @Type WHERE Id_Movies = @id", connection))
            {
                command.Parameters.AddWithValue("@Name", TxtName.Text);
                command.Parameters.AddWithValue("@Director", TxtDirector.Text);
                command.Parameters.AddWithValue("@Year", TxtYear.Text);
                command.Parameters.AddWithValue("@Type", TxtType.Text);
                command.Parameters.AddWithValue("@Id", TxtId.Text);

                try
                {
                    connection.Open();
                    command.ExecuteNonQuery();

                    MessageBox.Show("Edited successfully.");
                    Clear();
                }
                catch (SqlException ex)
                {
                    MessageBox.Show(ex.Message);
                    // TODO: Log the error somewhere
                }
            }
        }

        private void TxtSearch_Click(object sender, EventArgs e)
        {

            using (SqlConnection connection = CreateConnection())
            using (SqlCommand command = new SqlCommand("SELECT Name, Director, Year, Type FROM Movies WHERE Id_Movies = @id", connection))
            {
                command.Parameters.AddWithValue("@Id", TxtId.Text);

                try
                {
                    connection.Open();

                    using (SqlDataReader reader = command.ExecuteReader(CommandBehavior.CloseConnection))
                    {
                        if (!reader.Read())
                        {
                            MessageBox.Show("Movie not found.");
                        }
                        else
                        {
                            TxtName.Text = (string)reader["Name"];
                            TxtDirector.Text = (string)reader["Director"];
                            TxtYear.Text = (string)reader["Year"];
                            TxtType.Text = (string)reader["Type"];
                            
                        }
                    }
                }
                catch (SqlException ex)
                {
                    MessageBox.Show(ex.Message);
                    // TODO: Log the error somewhere
                }
            }
        }

        private void BtnDelete_Click(object sender, EventArgs e)
        {

            {
                using (SqlConnection connection = CreateConnection())
                using (SqlCommand command = new SqlCommand("Delete Movies WHERE Id_Movies = @Id", connection))
                {
                    command.Parameters.AddWithValue("@Name", TxtName.Text);
                    command.Parameters.AddWithValue("@Director", TxtDirector.Text);
                    command.Parameters.AddWithValue("@Year", TxtYear.Text);
                    command.Parameters.AddWithValue("@Type", TxtType.Text);
                    command.Parameters.AddWithValue("@Id", TxtId.Text);
                    

                    try
                    {
                        connection.Open();

                      

                        command.ExecuteNonQuery();

                        MessageBox.Show("Deleted successfully.");
                        Clear();
                    }
                    catch (SqlException ex)
                    {
                        MessageBox.Show(ex.Message);
                        // TODO: Log the error somewhere
                    }
                }
            }
 

        }

        private void pictureBox1_Click(object sender, EventArgs e)
        {

        }

        private void button1_Click(object sender, EventArgs e)
        {
            this.Close();
        }

        private void TxtPlay_Click(object sender, EventArgs e)
        {
            Form2 form2 = new Form2();
            form2.Show();
        }
    }
}

and this is form2 :

C#

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Windows.Forms;

namespace Film
{
    public partial class Form2 : Form
    {
        public Form2()
        {
            InitializeComponent();
        }

        private void button1_Click(object sender, EventArgs e)
        {
            this.Close();
        }
    }
}

Posted 26-Jul-15 0:42am

brandon1999

Updated 26-Jul-15 0:46am

v4

Add a Solution

3 solutions

Solution 3

Advice: Comment your code, it will help you in 1 years when you will try to read again that code.

Posted 26-Jul-15 1:19am

Patrice T

Comments

brandon1999 26-Jul-15 10:30am

ok thanks

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Wendelius · Accepted Answer · 2015-07-26T01:07:00

Solution 2

I fully agree with OriginalGriff, never ever store the passwords as clear text and always use parameters.

Few other things that haven't been mentioned yet:

in some places you use a centralized place for the connection string but not in all places, you should use only a single code fragment to define the connection string
don't hard code the connection string inside the program. Put it in a configuration file
there are a lot of extra parameters with the DELETE command, you use only id inside the SQL statement
you have empty event handlers, either add relevant code or remove the wiring

Posted 26-Jul-15 1:07am

Wendelius

v2

Comments

brandon1999 26-Jul-15 7:17am

ok can u fix the code for me . please i want to see how it solve.

Wendelius 26-Jul-15 7:22am

:) it's up to you to make the corrections, it's the only way to learn. But what is the part that's puzzling you?

You have successfully used parameters elsewhere, pat of the connections are okay but have a look at button1, there you repeat the connection string, why?

What comes to using configuration files, have a look at ConfigurationManager.AppSettings Property[^]

brandon1999 26-Jul-15 10:30am

ok i will fix it.

OriginalGriff · Accepted Answer · 2015-07-26T00:55:00

:laugh:
Hello again...

For starters, don't do it like that - you are committing two of the worst sins it is possible to commit (and a couple of poor ideas to add to it).

The first Sin is concatenating strings to form an SQL command:

C#

SqlStr = "   select LoginName,LoginPass ";
SqlStr = SqlStr + " from Login";
SqlStr = SqlStr + " where LoginName='" + TxtUserName.Text + "'  AND LoginPass='"+TxtPassword.Text+"' ";

I know you know about parameterized queries, because you use them elsewhere!
Concatenating strings is extremely dangerous: Google "Bobby Tables" and see what I mean. Users can damage or destroy your database if you do that...

The second Sin is storing passwords in clear text: this is bad...very bad...Code Crime[^]
There is a tip here which may help: Password Storage: How to do it.[^]

The first poor idea is adding strings together at all: learn to use a StringBuilder instead - it's a lot more memory efficient (remember, strings are immutable, so whenever you add to a string, you create a new one that is longer).

The second poor idea is using default names: Do yourself a favour, and stop using Visual Studio default names for everything - you may remember that "TextBox8" is the mobile number today, but when you have to modify it in three weeks time, will you then? Use descriptive names - "tbMobileNo" for example - and your code becomes easier to read, more self documenting, easier to maintain - and surprisingly quicker to code because Intellisense can get to to "tbMobile" in three keystrokes, where "TextBox8" takes thinking about and 8 keystrokes...you've done it most of the time, but not for your login for, or "Form2"...