你对此有何看法？ [英] What is your opinion about this?
问题描述
这是我的第一个程序（我认为这很无聊，但我必须练习很多）。你对此有何看法？可以让它更快？
这是我的登录代码：
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 :
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)
{
}
}
}
这是form1：
and this is form1 :
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();
}
}
}
这是form2：
and this is form2 :
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();
}
}
}
推荐答案
我完全同意OriginalGriff，永远不会存储密码作为明文并且总是使用参数。
很少有其他的东西还没有被提及：
- 在某些地方你使用集中的地方连接字符串而不是在所有地方，你应该只使用一个代码片段来定义连接字符串
- 不要硬编码程序中的连接字符串。把它放在配置文件中
- DELETE命令有很多额外的参数，你只在SQL语句中使用id
- 你有空事件处理程序，要么添加相关代码或删除接线
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
：笑：
你好再次...
对于初学者来说，不要这样做 - 你承诺犯下两个可能犯下的最严重的罪行（还有一些不好的想法可以加入）。
第一个Sin连接字符串形成一个SQL命令：
: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:
SqlStr = " select LoginName,LoginPass ";
SqlStr = SqlStr + " from Login";
SqlStr = SqlStr + " where LoginName='" + TxtUserName.Text + "' AND LoginPass='"+TxtPassword.Text+"' ";
我知道你知道参数化查询，因为你在其他地方使用它们！
连接字符串非常危险：GoogleBobby表并看看我的意思。如果你这样做，用户可能会损坏或破坏你的数据库...
第二个罪恶以明文形式存储密码：这很糟糕......非常糟糕...... 。代码犯罪 [ ^ ]
这里有一个提示可能会有所帮助：密码存储：如何做到这一点。 [ ^ ]
第一个糟糕的想法是将字符串加在一起：学会使用而是一个StringBuilder - 它的内存效率更高（请记住，字符串是不可变的，因此每当你添加一个字符串时，你创建一个更长的新字符串。）
第二个糟糕的想法是使用默认名称：帮自己一个忙，并停止使用Visual Studio默认名称 - 你可能还记得今天的TextBox8是手机号码，但是当你需要修改它时几周时间，你会吗？使用描述性名称 - 例如tbMobileNo - 您的代码变得更容易阅读，更自我记录，更易于维护 - 并且编码速度更快，因为Intellisense可以通过三次击键来tbMobile，其中TextBox8需要思考大概和8次击键......你大部分时间都是这样做的，但不是你的登录，或Form2......
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"...
建议：评论你的代码，它会在您尝试再次阅读该代码的1年内帮助您。
Advice: Comment your code, it will help you in 1 years when you will try to read again that code.
这篇关于你对此有何看法？的文章就介绍到这了，希望我们推荐的答案对大家有所帮助，也希望大家多多支持IT屋！
![代码犯罪](\"http://www.commitstrip.com/wp-content/uploads/2013/09/Strips-Erreur-au-pilori-001-650-finalenglish.jpg\"){kind=link}