HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Designing a Backspace key

Submitted by: @import:stackexchange-codereview··
0
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?

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:

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; its else clause 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 the if (textBox1.SelectionText.Length > 0) and else if (TxtIndex > 0) tests are both more specific.



  • textBox1.SelectedText.Length is better written as textBox1.SelectionLength.



  • The TxtIndex variable (which would be better named textIndex to be consistent with textBox1) 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.