How can I prevent DRY for this method?

Chris Anderson

Having each scope almost the same like below doesn't feel right at all. I'd rather have one scope where name, age i.e. are set than having 3 for each object of cat, dog and puppy. How can this code below be improved?

private void addAnimal_Click(object sender, EventArgs e)
{
var name = inputName.Text;
int age = int.Parse(inputAge.Text);
var favfood = inputFavfood.Text;
var breed = inputBreed.Text;
var typeOfAnimal = dropDownTypeOfAnimal.Text;
if (typeOfAnimal == "Cat")
{
Animal cat = new Cat();
cat.Name = name;
cat.Age = age;
cat.FavFood = favfood;
cat.Breed = breed;
cat.IsHungry = utility.ShuffleProbability();
Pets.Add(cat);
}
if (typeOfAnimal == "Dog")
{
Animal dog = new Dog();
dog.Name = name;
dog.Age = age;
dog.FavFood = favfood;
dog.Breed = breed;
dog.IsHungry = utility.ShuffleProbability();
Pets.Add(dog);
}
if (typeOfAnimal == "Puppy")
{
Animal puppy = new Puppy();
puppy.Name = name;
puppy.Age = age;
puppy.FavFood = favfood;
puppy.Breed = breed;
puppy.IsHungry = utility.ShuffleProbability();
Pets.Add(puppy);
}
}

4 Replies

Know the answer? Post it — somebody with the same question will find it here.

Laxmidhar Sahoo5y ago
hi
No need to take three classes just use one common calss named like petanimal and a distingushed filed
like pet type and add with the list
Sachin Singh5y ago
you can directly create the object based on string and in this way you will be free from if else.
private void addAnimal_Click(object sender, EventArgs e)
        {

            var name = inputName.Text;
            int age = int.Parse(inputAge.Text);
            var favfood = inputFavfood.Text;
            var breed = inputBreed.Text;
            var typeOfAnimal = dropDownTypeOfAnimal.Text;


              var assemblyName = "ConsoleTest"; // your namespace name

               Type t = Type.GetType(assemblyName+"."+typeOfAnimal );
                var obj= Activator.CreateInstance(t) as typeOfAnimal ;

                obj.Name = name;
                obj.Age = age;
                obj.FavFood = favfood;
                obj.Breed = breed;
                obj.IsHungry = utility.ShuffleProbability();
                Pets.Add(cat);


            }

        }
Note :- I have used Reflection.
Rijwan Ansari5y ago
In your case, I don't see any different properties based on if condition except object itself.

Simply,
private void addAnimal_Click(object sender, EventArgs e)
        {

            var name = inputName.Text;
            int age = int.Parse(inputAge.Text);
            var favfood = inputFavfood.Text;
            var breed = inputBreed.Text;
            var typeOfAnimal = dropDownTypeOfAnimal.Text;
Animal animal = null;

            if (typeOfAnimal == "Cat")
            {
                animal = new Cat();
            }
            else if (typeOfAnimal == "Dog")
            {
                animal = new Dog();
            }
            else if (typeOfAnimal == "Puppy")
            {
                animal = new Puppy();
            }
                animal.Name = name;
                animal.Age = age;
                animal.FavFood = favfood;
                animal.Breed = breed;
                animal.IsHungry = utility.ShuffleProbability();
                Pets.Add(animal);

        }
Addtionally, use if else or switch for better, as it will skip other conditions if one case is matched.
Regards
Kiran Mohanty5y ago
1. Animal animal= null;
2. if(typeOfAnimal == "cat"){
3. animal = new Cat();
4. }
5. elseif(typeOfAnimal == "dog"){
6. animal = new Dog();
7. }
8. else if(typeOfAnimal == "puppy"){
9. animal = new Puppy();
10. }
11. else {
12. animal = new Animal();
13. }
15. animal.Name = name;
16. animal.Age = age; // initialize all properties