Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

First problem you have is that you have mixed your code logic with UI. But tackling that issue could be too much at this point. I would recommend some reading about MVC/MVP patterns and you can start with What are MVP and MVC and what is the difference? What are MVP and MVC and what is the difference?

Another visible issue is repetition in your event handlers Button_Numeral_Click and Button_Hex_Click

I presume that each of that handlers is tied with specific buttons in your UI and only numeral buttons can trigger Button_Numeral_Click just like only hex buttons can trigger Button_Hex_Click That code can be simplified and it doesn't need a switch at all.

 #region Number Entries
 private void Button_Numeral_Click(object sender, EventArgs e)
 { 
 try
 {
 Button selected = (Button)sender;
 String NumVal = selected.Text;
 _TextBox_Output.Text += NumVal;
 }
 catch { MessageBox.Show("Not a valid entry"); }
 }
 #endregion
 #region Hex Entries
 private void Button_Hex_Click(object sender, EventArgs e)
 {
 try
 {
 Button selected = (Button)sender;
 String HexVal = selected.Text;
 _TextBox_Output.Text += HexVal;
 }
 catch { MessageBox.Show("Not a valid entry"); }
 }
 #endregion

Next step in simplifying above code would be removing try..catch block, since there are no errors you would actually be catching there. And one step further would be merging Button_Numeral_Click and Button_Hex_Click into same event handler, since their logic is identical

 private void Button_Num_Hex_Click(object sender, EventArgs e)
 {
 Button selected = (Button)sender;
 String NumHexVal = selected.Text;
 _TextBox_Output.Text += NumHexVal;
 }

First problem you have is that you have mixed your code logic with UI. But tackling that issue could be too much at this point. I would recommend some reading about MVC/MVP patterns and you can start with What are MVP and MVC and what is the difference?

Another visible issue is repetition in your event handlers Button_Numeral_Click and Button_Hex_Click

I presume that each of that handlers is tied with specific buttons in your UI and only numeral buttons can trigger Button_Numeral_Click just like only hex buttons can trigger Button_Hex_Click That code can be simplified and it doesn't need a switch at all.

 #region Number Entries
 private void Button_Numeral_Click(object sender, EventArgs e)
 { 
 try
 {
 Button selected = (Button)sender;
 String NumVal = selected.Text;
 _TextBox_Output.Text += NumVal;
 }
 catch { MessageBox.Show("Not a valid entry"); }
 }
 #endregion
 #region Hex Entries
 private void Button_Hex_Click(object sender, EventArgs e)
 {
 try
 {
 Button selected = (Button)sender;
 String HexVal = selected.Text;
 _TextBox_Output.Text += HexVal;
 }
 catch { MessageBox.Show("Not a valid entry"); }
 }
 #endregion

Next step in simplifying above code would be removing try..catch block, since there are no errors you would actually be catching there. And one step further would be merging Button_Numeral_Click and Button_Hex_Click into same event handler, since their logic is identical

 private void Button_Num_Hex_Click(object sender, EventArgs e)
 {
 Button selected = (Button)sender;
 String NumHexVal = selected.Text;
 _TextBox_Output.Text += NumHexVal;
 }

First problem you have is that you have mixed your code logic with UI. But tackling that issue could be too much at this point. I would recommend some reading about MVC/MVP patterns and you can start with What are MVP and MVC and what is the difference?

Another visible issue is repetition in your event handlers Button_Numeral_Click and Button_Hex_Click

I presume that each of that handlers is tied with specific buttons in your UI and only numeral buttons can trigger Button_Numeral_Click just like only hex buttons can trigger Button_Hex_Click That code can be simplified and it doesn't need a switch at all.

 #region Number Entries
 private void Button_Numeral_Click(object sender, EventArgs e)
 { 
 try
 {
 Button selected = (Button)sender;
 String NumVal = selected.Text;
 _TextBox_Output.Text += NumVal;
 }
 catch { MessageBox.Show("Not a valid entry"); }
 }
 #endregion
 #region Hex Entries
 private void Button_Hex_Click(object sender, EventArgs e)
 {
 try
 {
 Button selected = (Button)sender;
 String HexVal = selected.Text;
 _TextBox_Output.Text += HexVal;
 }
 catch { MessageBox.Show("Not a valid entry"); }
 }
 #endregion

Next step in simplifying above code would be removing try..catch block, since there are no errors you would actually be catching there. And one step further would be merging Button_Numeral_Click and Button_Hex_Click into same event handler, since their logic is identical

 private void Button_Num_Hex_Click(object sender, EventArgs e)
 {
 Button selected = (Button)sender;
 String NumHexVal = selected.Text;
 _TextBox_Output.Text += NumHexVal;
 }
Source Link

First problem you have is that you have mixed your code logic with UI. But tackling that issue could be too much at this point. I would recommend some reading about MVC/MVP patterns and you can start with What are MVP and MVC and what is the difference?

Another visible issue is repetition in your event handlers Button_Numeral_Click and Button_Hex_Click

I presume that each of that handlers is tied with specific buttons in your UI and only numeral buttons can trigger Button_Numeral_Click just like only hex buttons can trigger Button_Hex_Click That code can be simplified and it doesn't need a switch at all.

 #region Number Entries
 private void Button_Numeral_Click(object sender, EventArgs e)
 { 
 try
 {
 Button selected = (Button)sender;
 String NumVal = selected.Text;
 _TextBox_Output.Text += NumVal;
 }
 catch { MessageBox.Show("Not a valid entry"); }
 }
 #endregion
 #region Hex Entries
 private void Button_Hex_Click(object sender, EventArgs e)
 {
 try
 {
 Button selected = (Button)sender;
 String HexVal = selected.Text;
 _TextBox_Output.Text += HexVal;
 }
 catch { MessageBox.Show("Not a valid entry"); }
 }
 #endregion

Next step in simplifying above code would be removing try..catch block, since there are no errors you would actually be catching there. And one step further would be merging Button_Numeral_Click and Button_Hex_Click into same event handler, since their logic is identical

 private void Button_Num_Hex_Click(object sender, EventArgs e)
 {
 Button selected = (Button)sender;
 String NumHexVal = selected.Text;
 _TextBox_Output.Text += NumHexVal;
 }
lang-cs

AltStyle によって変換されたページ (->オリジナル) /