patterncsharpMinor
Shuffles male or females names based on Fisher Yates algorthim
Viewed 0 times
femalesmaleshufflesnamesalgorthimbasedyatesfisher
Problem
The code below has many
My question is, is there a way to make this code better? My task is to randomize names either male or female. The
```
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Windows;
using System.Windows.Controls;
using System.Windows.Data;
using System.Windows.Documents;
using System.Windows.Input;
using System.Windows.Media;
using System.Windows.Media.Imaging;
using System.Windows.Navigation;
using System.Windows.Shapes;
namespace PerfectName
{
///
/// Interaction logic for MainWindow.xaml
///
public partial class MainWindow : Window
{
ListBoxItem item1;
List MaleNamesThatStartWithA;
List MaleNamesThatStartWithB;
List MaleNamesThatStartWithC;
List MaleNamesThatStartWithD;
List MaleNamesThatStartWithE;
List MaleNamesThatStartWithF;
List MaleNamesThatStartWithG;
List MaleNamesThatStartWithH;
List MaleNamesThatStartWithI;
List MaleNamesThatStartWithJ;
List MaleNamesThatStartWithK;
List MaleNamesThatStartWithL;
List MaleNamesThatStartWithM;
List MaleNamesThatStartWithN;
List MaleNamesThatStartWithO;
List MaleNamesThatStartWithP;
List MaleNamesThatStartWithQ;
List MaleNamesThatStartWithR;
List MaleNamesThatStartWithS;
List MaleNamesThatStartWithT;
List MaleNamesThatStartWithU;
List MaleNamesThatStartWithV;
List MaleNamesThatStartWithW;
List MaleNamesThatStartWithX;
Lists<> but I'm not sure how to modify it. The only thing here that is an algorithm is the Fisher Yates shuffle algorithm. I am trying to put in good use an algorithm or just code better said to do this. It seems a bunch of code just used in repetition.My question is, is there a way to make this code better? My task is to randomize names either male or female. The
ListBox should stay like this. I don't want them to do much but just be there. Like the Country or birth date. How can improve this code to make it look cleaner and better? I am not concerned about the shuffler algorithm but more of the code that I have wrote down on here.```
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Windows;
using System.Windows.Controls;
using System.Windows.Data;
using System.Windows.Documents;
using System.Windows.Input;
using System.Windows.Media;
using System.Windows.Media.Imaging;
using System.Windows.Navigation;
using System.Windows.Shapes;
namespace PerfectName
{
///
/// Interaction logic for MainWindow.xaml
///
public partial class MainWindow : Window
{
ListBoxItem item1;
List MaleNamesThatStartWithA;
List MaleNamesThatStartWithB;
List MaleNamesThatStartWithC;
List MaleNamesThatStartWithD;
List MaleNamesThatStartWithE;
List MaleNamesThatStartWithF;
List MaleNamesThatStartWithG;
List MaleNamesThatStartWithH;
List MaleNamesThatStartWithI;
List MaleNamesThatStartWithJ;
List MaleNamesThatStartWithK;
List MaleNamesThatStartWithL;
List MaleNamesThatStartWithM;
List MaleNamesThatStartWithN;
List MaleNamesThatStartWithO;
List MaleNamesThatStartWithP;
List MaleNamesThatStartWithQ;
List MaleNamesThatStartWithR;
List MaleNamesThatStartWithS;
List MaleNamesThatStartWithT;
List MaleNamesThatStartWithU;
List MaleNamesThatStartWithV;
List MaleNamesThatStartWithW;
List MaleNamesThatStartWithX;
Solution
The logic part of your code should definitely be re-written, but since it's one big mess I'm not going to touch that. See whether you really need that many options. You probably don't. I might expand on this anyway later on, since code like this hurts.
However, I can tell you this smells:
For one, did you notice the list of
So why don't we simply write those lists into more lists?
There. Done. I just threw out 50 lines of code.
Instead of adding something to
Instead of adding something to
The following makes me think you really wanted to create a
You really don't want to name a variable
So how about a
Now, all of a sudden, you can do this:
All at once!
Keep in mind we suddenly changed the type of what we're adding to the
However, I can tell you this smells:
List MaleNamesThatStartWithA;
List MaleNamesThatStartWithB;
List MaleNamesThatStartWithC;
List MaleNamesThatStartWithD;
List MaleNamesThatStartWithE;
List MaleNamesThatStartWithF;
List MaleNamesThatStartWithG;
List MaleNamesThatStartWithH;
List MaleNamesThatStartWithI;
List MaleNamesThatStartWithJ;
List MaleNamesThatStartWithK;
List MaleNamesThatStartWithL;
List MaleNamesThatStartWithM;
List MaleNamesThatStartWithN;
List MaleNamesThatStartWithO;
List MaleNamesThatStartWithP;
List MaleNamesThatStartWithQ;
List MaleNamesThatStartWithR;
List MaleNamesThatStartWithS;
List MaleNamesThatStartWithT;
List MaleNamesThatStartWithU;
List MaleNamesThatStartWithV;
List MaleNamesThatStartWithW;
List MaleNamesThatStartWithX;
List MaleNamesThatStartWithY;
List MaleNamesThatStartWithZ;
List FemaleNamesThatStartWithA;
List FemaleNamesThatStartWithB;
List FemaleNamesThatStartWithC;
List FemaleNamesThatStartWithD;
List FemaleNamesThatStartWithE;
List FemaleNamesThatStartWithF;
List FemaleNamesThatStartWithG;
List FemaleNamesThatStartWithH;
List FemaleNamesThatStartWithI;
List FemaleNamesThatStartWithJ;
List FemaleNamesThatStartWithK;
List FemaleNamesThatStartWithL;
List FemaleNamesThatStartWithM;
List FemaleNamesThatStartWithN;
List FemaleNamesThatStartWithO;
List FemaleNamesThatStartWithP;
List FemaleNamesThatStartWithQ;
List FemaleNamesThatStartWithR;
List FemaleNamesThatStartWithS;
List FemaleNamesThatStartWithT;
List FemaleNamesThatStartWithU;
List FemaleNamesThatStartWithV;
List FemaleNamesThatStartWithW;
List FemaleNamesThatStartWithX;
List FemaleNamesThatStartWithY;
List FemaleNamesThatStartWithZ;For one, did you notice the list of
FemaleNames is an exact copy of the list of MaleNames? Did you also notice how there's exactly 26 letters in the alphabet and this is not likely to change? Did you also notice the location of the letter in the alphabet is not likely to change?So why don't we simply write those lists into more lists?
List> MaleNames;
List> FemaleNames;There. Done. I just threw out 50 lines of code.
Instead of adding something to
MaleNamesThatStartWithA, you now add something to MaleNames[12].Instead of adding something to
FemaleNamesThatStartWithB, you now add something to FemaleNames[1].The following makes me think you really wanted to create a
Person object instead:item1 = new ListBoxItem();
item1.Content = "German";
ListBoxItem item2 = new ListBoxItem();
item2.Content = "American";
listBox1.Items.Add(item1);
listBox1.Items.Add(item2);
ListBoxItem sign1 = new ListBoxItem();
sign1.Content = "Cancer";
ListBoxItem sign2 = new ListBoxItem();
sign2.Content = "Gemini";
listBox2.Items.Add(sign1);
listBox2.Items.Add(sign2);
ListBoxItem bday1 = new ListBoxItem();
bday1.Content = "1900";
ListBoxItem bday2 = new ListBoxItem();
bday2.Content = "1901";
listBox3.Items.Add(bday1);
listBox3.Items.Add(bday2);You really don't want to name a variable
item1. It's ambiguous.So how about a
Class containing all the relevant information of a Person?class Person
{
public Person(string fn, string ln, DateTime dob, string nat, string sig) //Parameterized constructor
{
FirstName = fn;
LastName = ln;
DateOfBirth = dob;
Nationality = nat;
Sign = sig;
}
public string FirstName { get; set; }
public string LastName { get; set; }
public DateTime DateOfBirth { get; set; }
public string Nationality { get; set; }
public string Sign { get; set; }
}Now, all of a sudden, you can do this:
MaleNames[3].Add(new Person { "Doug", "Howitzer", new DateTime(1974, 7, 10, 7, 10, 24), "German", "Zodiac" });All at once!
Keep in mind we suddenly changed the type of what we're adding to the
List, so now we instantiate like this:List> MaleNames;
List> FemaleNames;Code Snippets
List<string> MaleNamesThatStartWithA;
List<string> MaleNamesThatStartWithB;
List<string> MaleNamesThatStartWithC;
List<string> MaleNamesThatStartWithD;
List<string> MaleNamesThatStartWithE;
List<string> MaleNamesThatStartWithF;
List<string> MaleNamesThatStartWithG;
List<string> MaleNamesThatStartWithH;
List<string> MaleNamesThatStartWithI;
List<string> MaleNamesThatStartWithJ;
List<string> MaleNamesThatStartWithK;
List<string> MaleNamesThatStartWithL;
List<string> MaleNamesThatStartWithM;
List<string> MaleNamesThatStartWithN;
List<string> MaleNamesThatStartWithO;
List<string> MaleNamesThatStartWithP;
List<string> MaleNamesThatStartWithQ;
List<string> MaleNamesThatStartWithR;
List<string> MaleNamesThatStartWithS;
List<string> MaleNamesThatStartWithT;
List<string> MaleNamesThatStartWithU;
List<string> MaleNamesThatStartWithV;
List<string> MaleNamesThatStartWithW;
List<string> MaleNamesThatStartWithX;
List<string> MaleNamesThatStartWithY;
List<string> MaleNamesThatStartWithZ;
List<string> FemaleNamesThatStartWithA;
List<string> FemaleNamesThatStartWithB;
List<string> FemaleNamesThatStartWithC;
List<string> FemaleNamesThatStartWithD;
List<string> FemaleNamesThatStartWithE;
List<string> FemaleNamesThatStartWithF;
List<string> FemaleNamesThatStartWithG;
List<string> FemaleNamesThatStartWithH;
List<string> FemaleNamesThatStartWithI;
List<string> FemaleNamesThatStartWithJ;
List<string> FemaleNamesThatStartWithK;
List<string> FemaleNamesThatStartWithL;
List<string> FemaleNamesThatStartWithM;
List<string> FemaleNamesThatStartWithN;
List<string> FemaleNamesThatStartWithO;
List<string> FemaleNamesThatStartWithP;
List<string> FemaleNamesThatStartWithQ;
List<string> FemaleNamesThatStartWithR;
List<string> FemaleNamesThatStartWithS;
List<string> FemaleNamesThatStartWithT;
List<string> FemaleNamesThatStartWithU;
List<string> FemaleNamesThatStartWithV;
List<string> FemaleNamesThatStartWithW;
List<string> FemaleNamesThatStartWithX;
List<string> FemaleNamesThatStartWithY;
List<string> FemaleNamesThatStartWithZ;List<List<string>> MaleNames;
List<List<string>> FemaleNames;item1 = new ListBoxItem();
item1.Content = "German";
ListBoxItem item2 = new ListBoxItem();
item2.Content = "American";
listBox1.Items.Add(item1);
listBox1.Items.Add(item2);
ListBoxItem sign1 = new ListBoxItem();
sign1.Content = "Cancer";
ListBoxItem sign2 = new ListBoxItem();
sign2.Content = "Gemini";
listBox2.Items.Add(sign1);
listBox2.Items.Add(sign2);
ListBoxItem bday1 = new ListBoxItem();
bday1.Content = "1900";
ListBoxItem bday2 = new ListBoxItem();
bday2.Content = "1901";
listBox3.Items.Add(bday1);
listBox3.Items.Add(bday2);class Person
{
public Person(string fn, string ln, DateTime dob, string nat, string sig) //Parameterized constructor
{
FirstName = fn;
LastName = ln;
DateOfBirth = dob;
Nationality = nat;
Sign = sig;
}
public string FirstName { get; set; }
public string LastName { get; set; }
public DateTime DateOfBirth { get; set; }
public string Nationality { get; set; }
public string Sign { get; set; }
}MaleNames[3].Add(new Person { "Doug", "Howitzer", new DateTime(1974, 7, 10, 7, 10, 24), "German", "Zodiac" });Context
StackExchange Code Review Q#141582, answer score: 7
Revisions (0)
No revisions yet.