patterncsharpMinor
Designing a Backspace key
Viewed 0 times
designingkeybackspace
Problem
I have designed a Backspace button (part of a keyboard which I am designing) in windows forms and it works perfect with no errors (the code works very good). This button deletes what is inside the textbox.
I just want to see other people's views and what they think about my code, so because I am new to c# I think there might be other better ways to design Backspace button. Can you please see if there are any silly unprofessional mistakes in my code?
I just want to see other people's views and what they think about my code, so because I am new to c# I think there might be other better ways to design Backspace button. Can you please see if there are any silly unprofessional mistakes in my code?
private void buttonDelete_Click(object sender, EventArgs e)
{
textBox1.Focus(); // set the textbox focused.
if (textBox1.SelectionStart >= 0) //check if there is text to delete
{
int TxTindex = textBox1.SelectionStart; // save the index position
if (textBox1.SelectedText.Length > 0) //// check if there is selected text
{
textBox1.SelectedText = "";
}
else
{
if (TxTindex >0 && TxTindex == textBox1.Text.Length) // check if there is text in texbox and check if index is at the end of text.
{
textBox1.Text = textBox1.Text.Remove(textBox1.Text.Length - 1, 1);
textBox1.SelectionStart = textBox1.Text.Length;
}
else // is the index is in between the text?
{
if (TxTindex > 0) // check if there is text in texbox
{
textBox1.Text = textBox1.Text.Remove(TxTindex - 1, 1);
textBox1.SelectionStart = TxTindex - 1; // to set the cursor position after the deleted number between the text.
}
else
{
return;
}
}
}
}
else
{
return;
}
}Solution
Through a series of mechanical transformations, it's possible to simplify the code down to this:
It's now simple enough that the code would be better without comments.
How I got there:
The code so far:
private void buttonDelete_Click(object sender, EventArgs e)
{
textBox1.Focus();
if (textBox1.SelectionLength > 0)
{
textBox1.SelectedText = "";
}
else if (textBox1.SelectionStart > 0)
{
textBox1.Text = textBox1.Text.Remove(--textBox1.SelectionStart, 1);
}
}It's now simple enough that the code would be better without comments.
How I got there:
- Both of your
else { return; }are superfluous.
- The
if (TxTindex >0 && TxTindex == textBox1.Text.Length)case is a redundant special case; itselseclause contains exactly the same code.
The code so far:
private void buttonDelete_Click(object sender, EventArgs e)
{
textBox1.Focus(); // set the textbox focused.
if (textBox1.SelectionStart >= 0) //check if there is text to delete
{
int TxTindex = textBox1.SelectionStart; // save the index position
if (textBox1.SelectedText.Length > 0) //// check if there is selected text
{
textBox1.SelectedText = "";
}
else if (TxTindex > 0) // check if there is text in texbox
{
textBox1.Text = textBox1.Text.Remove(TxTindex - 1, 1);
textBox1.SelectionStart = TxTindex - 1; // to set the cursor position after the deleted number between the text.
}
}
}- The outer
if (textBox1.SelectionStart >= 0)is redundant, since theif (textBox1.SelectionText.Length > 0)andelse if (TxtIndex > 0)tests are both more specific.
textBox1.SelectedText.Lengthis better written astextBox1.SelectionLength.
- The
TxtIndexvariable (which would be better namedtextIndexto be consistent withtextBox1) can be eliminated.
Code Snippets
private void buttonDelete_Click(object sender, EventArgs e)
{
textBox1.Focus();
if (textBox1.SelectionLength > 0)
{
textBox1.SelectedText = "";
}
else if (textBox1.SelectionStart > 0)
{
textBox1.Text = textBox1.Text.Remove(--textBox1.SelectionStart, 1);
}
}private void buttonDelete_Click(object sender, EventArgs e)
{
textBox1.Focus(); // set the textbox focused.
if (textBox1.SelectionStart >= 0) //check if there is text to delete
{
int TxTindex = textBox1.SelectionStart; // save the index position
if (textBox1.SelectedText.Length > 0) //// check if there is selected text
{
textBox1.SelectedText = "";
}
else if (TxTindex > 0) // check if there is text in texbox
{
textBox1.Text = textBox1.Text.Remove(TxTindex - 1, 1);
textBox1.SelectionStart = TxTindex - 1; // to set the cursor position after the deleted number between the text.
}
}
}Context
StackExchange Code Review Q#111235, answer score: 2
Revisions (0)
No revisions yet.