4
\$\begingroup\$

I have written some code for inserting a label at runtime that has its content set to a string array into a datagrid row. All of this will initiate when certain radio buttons are checked. The code is working perfectly fine, but I need to improve this code as I am learning C#, WPF and datagrid. I know there can be a certain way to improve this code. This code will be a nightmare when there are 50 radio buttons. Can it be improved?

XAML Code:

<Grid> <RadioButton x:Name="rb_1" Content="RadioButton" HorizontalAlignment="Left" Margin="351,85,0,0" VerticalAlignment="Top" GroupName="1" /> <RadioButton x:Name="rb_2" Content="RadioButton" HorizontalAlignment="Left" Margin="351,105,0,0" VerticalAlignment="Top" GroupName="1"/> <RadioButton x:Name="rb_3" Content="RadioButton" HorizontalAlignment="Left" Margin="351,120,0,0" VerticalAlignment="Top" GroupName="1" /> <RadioButton Content="RadioButton" HorizontalAlignment="Left" Margin="351,159,0,0" VerticalAlignment="Top" GroupName="2" /> <RadioButton x:Name="rb_4" Content="RadioButton" HorizontalAlignment="Left" Margin="351,179,0,0" VerticalAlignment="Top" GroupName="2"/> <RadioButton Content="RadioButton" HorizontalAlignment="Left" Margin="351,199,0,0" VerticalAlignment="Top" GroupName="2" /> <Button Content="Button" HorizontalAlignment="Left" Margin="713,60,0,0" VerticalAlignment="Top" Width="75" Click="Button_Click_2"/> <DataGrid x:Name="datagrid_" HorizontalAlignment="Left" Margin="549,85,0,0" VerticalAlignment="Top" Height="253" Width="399" /> <RadioButton Content="RadioButton" HorizontalAlignment="Left" Margin="351,226,0,0" VerticalAlignment="Top" GroupName="3" /> <RadioButton x:Name="rb_6" Content="RadioButton" HorizontalAlignment="Left" Margin="351,246,0,0" VerticalAlignment="Top" GroupName="3"/> <RadioButton Content="RadioButton" HorizontalAlignment="Left" Margin="351,266,0,0" VerticalAlignment="Top" GroupName="3" /> <RadioButton Content="RadioButton" HorizontalAlignment="Left" Margin="351,298,0,0" VerticalAlignment="Top" GroupName="4" /> <RadioButton x:Name="rb_8" Content="RadioButton" HorizontalAlignment="Left" Margin="351,318,0,0" VerticalAlignment="Top" GroupName="4"/> <RadioButton Content="RadioButton" HorizontalAlignment="Left" Margin="351,338,0,0" VerticalAlignment="Top" GroupName="4" /> 

Code Behind:

public partial class MainWindow : Window { public MainWindow() { InitializeComponent(); } DataTable dt; DataRow dr; string[] str = new string[4]; int location = 0; int count = 0; private void Window_Loaded(object sender, RoutedEventArgs e) { dt = new DataTable("emp"); DataColumn dc1 = new DataColumn("Factors", typeof(string)); DataColumn dc2 = new DataColumn("Non_Compliant", typeof(string)); dt.Columns.Add(dc1); dt.Columns.Add(dc2); datagrid_.ItemsSource = dt.DefaultView; } private void Button_Click_2(object sender, RoutedEventArgs e) { if (count >= 1) { datagrid_.ItemsSource = dt.DefaultView; dt.Clear(); } str[0] = "Load Path1"; str[1] = "Load Path2"; str[2] = "Load Path3"; str[3] = "Load Path4"; int j = 0; if (rb_2.IsChecked == true) { j = 0; int k = 0; dr = dt.NewRow(); Label label = new Label(); label.Height = 28; label.Width = 100; label.HorizontalAlignment = HorizontalAlignment.Center; label.VerticalAlignment = VerticalAlignment.Center; label.Content = str[j]; dr[k] = label.Content; dt.Rows.Add(dr); datagrid_.ItemsSource = dt.DefaultView; location += 34; } if (rb_4.IsChecked == true) { j = 1; int k = 0; dr = dt.NewRow(); Label label = new Label(); label.Height = 28; label.Width = 100; label.HorizontalAlignment = HorizontalAlignment.Center; label.VerticalAlignment = VerticalAlignment.Center; label.Content = str[j]; dr[k] = label.Content; dt.Rows.Add(dr); datagrid_.ItemsSource = dt.DefaultView; location += 34; } if (rb_6.IsChecked == true) { j = 2; int k = 0; dr = dt.NewRow(); Label label = new Label(); label.Height = 28; label.Width = 100; label.HorizontalAlignment = HorizontalAlignment.Center; label.VerticalAlignment = VerticalAlignment.Center; label.Content = str[j]; dr[k] = label.Content; dt.Rows.Add(dr); datagrid_.ItemsSource = dt.DefaultView; location += 34; } if (rb_8.IsChecked == true) { j = 3; int k = 0; dr = dt.NewRow(); Label label = new Label(); label.Height = 28; label.Width = 100; label.HorizontalAlignment = HorizontalAlignment.Center; label.VerticalAlignment = VerticalAlignment.Center; label.Content = str[j]; dr[k] = label.Content; dt.Rows.Add(dr); datagrid_.ItemsSource = dt.DefaultView; location += 34; } count++; } } 
\$\endgroup\$
2
  • \$\begingroup\$Welcome to CodeReview, Vanadium90. Hope you get some fine answers.\$\endgroup\$
    – Legato
    CommentedApr 12, 2015 at 15:51
  • \$\begingroup\$looking forward :)\$\endgroup\$CommentedApr 12, 2015 at 15:54

1 Answer 1

3
\$\begingroup\$

Don't do this:

DataTable dt; DataRow dr; string[] str = new string[4]; int location = 0; int count = 0; 

Always use explicit access modifiers.

Also, do any of those fields really need to be global?


Now, the real problem is that you don't follow MVVM. A lot of my other issues follow from that.

For instance:

<Button Content="Button" HorizontalAlignment="Left" Margin="713,60,0,0" VerticalAlignment="Top" Width="75" Click="Button_Click_2"/> 

You shouldn't use the Click of a button; instead you should bind it to a command.

You also use the name of your DataGrid in your code behind, datagrid_. That is a meaningless name that doesn't follow any naming guideline. Quite frankly, I don't think I've used the Name of a XAML object more than a handful of times in the years I did Silverlight development: whenever possible you should bind to a source.

I also see a lot of Margin properties being used in very specific ways. I would urge you to look into the various layout possibilities of XAML and apply those. Don't work pixel-perfect, it's pointless IMHO and just a maintenance nightmare.

You XAML code looks like it's drag & drop. Which is easy to use I guess, but I'd urge you to abandon the visual editor and code your XAML "by hand".


A quick solution for now: look at the code that you repeat over and over, i.e. most of this:

 int k = 0; dr = dt.NewRow(); Label label = new Label(); label.Height = 28; label.Width = 100; label.HorizontalAlignment = HorizontalAlignment.Center; label.VerticalAlignment = VerticalAlignment.Center; label.Content = str[j]; dr[k] = label.Content; dt.Rows.Add(dr); datagrid_.ItemsSource = dt.DefaultView; location += 34; 

Once you start to copy-paste code and simply change one or two things, you know that's a sign you need to move it to a method that will accept the necessary parameters.

\$\endgroup\$
1
  • \$\begingroup\$you know what sir. . I love u :) thanks and thanks alot for the helpful guideline and review. I will improve it in sha ALLAH :) any good book or tutorial on databinding that u recommend please?Eager to learn.\$\endgroup\$CommentedApr 12, 2015 at 17:49

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.