-1
\$\begingroup\$

Since i'm still learning C# i've made an exercise but it seems like i'm using the same code over and over again. I do realize I have to use a helper method to get rid of the 'useless' code.

How can I put my for loops into a helper method ?

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace NewTryoutExamen
{
    class Ploeg
    {
        private List<Werknemer> werknemers = new List<Werknemer>();

        public void VoegWerknemerToe(Werknemer w)
        {
            for (int i = 0; i < werknemers.Count; i++)
            {
                if (w.Id == werknemers[i].Id)
                    throw new DuplicateWaitObjectException();
            }

            werknemers.Add(w);
        }

        public uint Prestatie(uint werknemerId, uint hoeveelheid)
        {
            for (int i = 0; i < werknemers.Count; i++)
            {
                if (werknemerId == werknemers[i].Id)
                    return werknemers[i].VoegVerkopenToe(hoeveelheid);
            }
            throw new KeyNotFoundException();
        }


        public uint GeefAantalEenheden(uint werknemerId)
        {
            for (int i = 0; i < werknemers.Count; i++)
            {
                if (werknemerId == werknemers[i].Id)
                    return werknemers[i].AantalEenheden;
            }
            throw new KeyNotFoundException();
        }

        public double GeefTotaalLoon(uint werknemerId)
        {
            for (int i = 0; i < werknemers.Count; i++)
            {
                if (werknemerId == werknemers[i].Id)
                    return werknemers[i].TotaalLoon();
            }
            throw new KeyNotFoundException();
        }

        public Werknemer ToonWerknemerDetails(uint werknemerId)
        {
            for (int i = 0; i < werknemers.Count; i++)
            {
                if (werknemerId == werknemers[i].Id)
                    return werknemers[i];
            }
            throw new KeyNotFoundException(); 
        }

        public double Basisloon(uint werknemerId)
        {
            for (int i = 0; i < werknemers.Count; i++)
            {
                if (werknemerId == werknemers[i].Id)
                {
                    return werknemers[i].Basisloon;
                }
            }
            throw new KeyNotFoundException();
        }


        public double Commissie(uint werknemerId)
        {
            for (int i = 0; i < werknemers.Count; i++)
            {
                if (werknemerId == werknemers[i].Id)
                {
                    return werknemers[i].Commissie;
                }
            }
            throw new KeyNotFoundException();
        }


        public override string ToString()
        {
            string res = "";
            string Id;

            for (int i = 0; i < werknemers.Count; i++)
            {
                Id = string.Format("{0:0000}", werknemers[i].Id);
                res += Id + " - " + werknemers[i].Naam + " " + werknemers[i].Voornaam + " ( eenheden: " + werknemers[i].AantalEenheden + ")" + Environment.NewLine;
            }
            return res;
        }
    }
}
\$\endgroup\$
1

2 Answers 2

1
\$\begingroup\$

You should definitely look into LINQ. It saves alot of space.

A few reworks;

for (int i = 0; i < werknemers.Count; i++)
{
    if (w.Id == werknemers[i].Id)
       throw new DuplicateWaitObjectException();
}

becomes

if(werknemers.Any(wn => w.Id == wn.Id)
{
    throw new DuplicateWaitObjectException();
}

And

for (int i = 0; i < werknemers.Count; i++)
{
    if (werknemerId == werknemers[i].Id)
        return werknemers[i].VoegVerkopenToe(hoeveelheid);
}

becomes

return werknemers.Where(w => w.Id == werknemerId).FirstOrDefault().VoegVerkopenToe(hoeveelheid);

I'll leave the rest as an excercise :)

\$\endgroup\$
4
  • \$\begingroup\$ I got an error with the next one ... return werknemers.Where(w => werknemerId == w.Id).FirstOrDefault().AantalEenheden; throw new KeyNotFoundException(); \$\endgroup\$
    – Awa
    May 18, 2015 at 14:18
  • \$\begingroup\$ @Awa it helps if you tell us what error it is :) \$\endgroup\$
    – Dion V.
    May 18, 2015 at 14:32
  • \$\begingroup\$ Object reference not set to an instance of an object \$\endgroup\$
    – Awa
    May 18, 2015 at 14:47
  • \$\begingroup\$ @Awa I was hoping you'd figure that out by yourself, therefore I am leaving it for exercise. A big hint: FirstOrDefault(). \$\endgroup\$
    – Dion V.
    May 18, 2015 at 14:49
4
\$\begingroup\$

There is a pre-built method for testing if any element matches a condition:

if(werknemers.Any(werknemer => w.Id == werknemer.Id)){
    throw new DuplicateWaitObjectException();
}
werknemers.Add(w);

However you look up every werknemer a lot based on the ID. You should use a Dictionary<uint, Werknemer> instead which includes methods to check if a key is already used and a way to get the object back with the key:

public double Commissie(uint werknemerId)
{
    if(werknemers.ContainsKey(werknemerId))
        return werknemers[werknemerId].Commissie;
    }
    throw new KeyNotFoundException();
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.