views:

186

answers:

7

I have a data class Student, and I have an aggregate class Students. Student has two properties of type string : Name and City.

what I want to do is have the option to choose what property to iterate using the foreach mechanism.

The code I wrote works and it's also readable and nice-looking. The main issue is performance : the line in which I use the yield keyword is probably not very efficient , but the question is how much ? is it dramatic performance hit ?

Is there a better way to achieve this functionality? (added: i don't want to allow someone to modify the returning Student objects, so all the Linq solutions proposed aren't good here. To make it clearer , I want:
property iteration + foreach mechanism integration + Student class and the list of students are readonly. how can i achieve that ?)

static void Main(string[] args)
    {           
        Students students = new Students();

        students.AddStudent(new Student { Age = 20, Name = "Stud1" , City="City1" });
        students.AddStudent(new Student { Age = 46, Name = "Stud2" , City="City2"});
        students.AddStudent(new Student { Age = 32, Name = "Stud3" , City="City3" });
        students.AddStudent(new Student { Age = 34, Name = "Stud4" , City="city4" });

        students.PropertyToIterate = eStudentProperty.City;
        foreach (string studentCity in students)
        {
            Console.WriteLine(studentcity);
        }

        students.PropertyToIterate = eStudentProperty.Name;
        foreach (string studentName in students)
        {
            Console.WriteLine(studentName);
        }

    }

public class Students :IEnumerable<object>
{
    private List<Student> m_Students = new List<Student>();

    private eStudentProperty m_PropertyToIterate = eStudentProperty.Name;

    public eStudentProperty PropertyToIterate
    {
        get { return m_PropertyToIterate; }
        set { m_PropertyToIterate = value; }
    }

    public void AddStudent(Student i_Student)
    {
        m_Students.Add(i_Student);
    }

    public IEnumerator<object> GetEnumerator()
    {            
        for (int i = 0; i < m_Students.Count; ++i)
        {
            yield return (object)m_Students[i].GetType().GetProperty(PropertyToIterate.ToString()).GetValue(m_Students[i], null);
        }            
    }

    IEnumerator IEnumerable.GetEnumerator()
    {
        throw new NotImplementedException();
    }
}

public enum eStudentProperty
{
    Name,
    Age,
    City
} 

public class Student
{
    public string Name { get; set; }

    public string City { get; set; }

    public int Age { get; set; }
}
+4  A: 

You could use lambda's to do something similar

PropertyToIterate would then take a Func<Student, object> that you can set this way:

Students.PropertyToIterate = student => student.City;

GetEnumerator can be implemented with linq like this:

return from student in m_Students select PropertyToIterate(student);

Or without linq like this

foreach(var student in students)
{
  yield return PropertyToIterate(student);
}
Mendelt
i made my post clearer. is there a solution now ?
Yaron Levi
+10  A: 

Why not simply get the properties with Linq and keep the original Enumeration of the students so you can iterate all the students in the Students class.

    foreach (string studentCity in students.Select(s => s.City))
    {
        Console.WriteLine(studentcity);
    }
    ...
    foreach (string studentName in students.Select(s => s.Name))
    {
        Console.WriteLine(studentName);
    }
Torbjörn Hansson
+1: the ogirinal solution looks horrible without explanation.
Grozz
+6  A: 

In response to your edit, how about something like this...

Students students = new Students();
students.AddStudent(new Student { Age = 20, Name = "Stud1", City = "City1" });
students.AddStudent(new Student { Age = 46, Name = "Stud2", City = "City2" });
students.AddStudent(new Student { Age = 32, Name = "Stud3", City = "City3" });
students.AddStudent(new Student { Age = 34, Name = "Stud4", City = "city4" });

foreach (int studentAge in students.EnumerateBy(StudentProperty.Age))
{
    Console.WriteLine(studentAge);
}

foreach (string studentName in students.EnumerateBy(StudentProperty.Name))
{
    Console.WriteLine(studentName);
}

foreach (string studentCity in students.EnumerateBy(StudentProperty.City))
{
    Console.WriteLine(studentCity);
}

// ...

public class Students
{
    private List<Student> _students = new List<Student>();

    public void AddStudent(Student student)
    {
        _students.Add(student);
    }

    public IEnumerable<T> EnumerateBy<T>(StudentProperty<T> property)
    {
        return _students.Select(property.Selector);
    }
}

public static class StudentProperty
{
    public static readonly StudentProperty<int> Age =
        new StudentProperty<int>(s => s.Age);

    public static readonly StudentProperty<string> Name =
        new StudentProperty<string>(s => s.Name);

    public static readonly StudentProperty<string> City =
        new StudentProperty<string>(s => s.City);
}

public sealed class StudentProperty<T>
{
    internal Func<Student, T> Selector { get; private set; }

    internal StudentProperty(Func<Student, T> selector)
    {
        Selector = selector;
    }
}
LukeH
i made my post clearer. is there a solution now ?
Yaron Levi
Thanks ! .... looks like it answers all the requirements, but it looks complicated , i need to read it more thoroughly .
Yaron Levi
A: 

You may be experiencing performance loss due to repeatedly reflecting the type every iteration. To avoid unnecessary calls to GetType(), pull it out of the loop. Also, since the type is known (e.g., Student), you can gain some compile time efficiency using typeof.

public IEnumerator<object> GetEnumerator()
{    
    var property = typeof(Student).GetProperty(PropertyToIterate.ToString());
    for (int i = 0; i < m_Students.Count; ++i)
    {
        yield return property.GetValue(m_Students[i], null);
    }            
}

On an aside, you can satisfy the non-generic GetEnumerator() by returning the generic version.

IEnumerator IEnumerable.GetEnumerator()
{
    return GetEnumerator<object>();
}
kbrimington
`yield return (string)property.GetValue(m_Students[i], null);` is not functionally correct; it will not work for `Age`, which is an `int`. You either need to remove the cast, or to call `.ToString()` in which case you can improve things by changing the enumerator type to `IEnumerator<string>`.
Ani
@Ani: Fair enough. I had only reproduced the OP's call. Edited now to ignore the typecast.
kbrimington
@kbrimington As I understand it, the object's metadata already contains the Type information, so `GetType()` is a very, very small performance hit. That having been said, I don't see a reason to use it here.
George Stocker
@George - It would be interesting to see real benchmarks to measure the difference. To be honest, I prefer the Linq-to-Sql approaches anyway, but wanted to address the OP's question directly regarding perceived efficiency issues in the `yield` statement.
kbrimington
+1  A: 

if student can be a struct then that will handle the readonly part of the student item. Otherwise, just make the properties of the Student class set in the constructor and remove the public set.

edit: okay, no struct. changing Student to a class

public class Students : ReadOnlyCollection<Student>
{
    public Students(IList<Student> students) : base(students)
    {}

    public IEnumerable<string> Names
    {
        get { return this.Select(x => x.Name); }
    }

    public IEnumerable<string> Cities
    {
        get { return this.Select(x => x.City); }
    }
}

public class Student 
{
              public Student(string name, string city, int age)
              {
                  this.Name = name;
                  this.City = city;
                  this.Age = age;
              }
    public string Name { get; private set; } 

    public string City { get; private set; } 

    public int Age { get; private set; } 
}

class Program
{
    static void Main(string[] args)
    {
        List<Student> students = new List<Student>();
        students.Add(new Student("Stud1", "City1",20));
        students.Add(new Student("Stud2", "City2",46));
        students.Add(new Student("Stud3", "City3",66));
        students.Add(new Student("Stud4","city4", 34));


        Students readOnlyStudents = new Students(students);

        foreach (string studentCity in readOnlyStudents.Cities)
        {
            Console.WriteLine(studentCity);
        }

        foreach (string studentName in readOnlyStudents.Names)
        {
            Console.WriteLine(studentName);
        }
    } 
}
internet man
I don't want Student to be a struct. It doesn't fit in my app.
Yaron Levi
+1  A: 

What about adding an interface?

public interface IStudent
{
    public string Name { get; }
    public string City { get; }
    public int Age { get; }
}

public class Student : IStudent
{
    ...
}

public class Students : IEnumerable<IStudent>
{
    ...
}

Then you could use the LINQ solution and the Student objects can not be altered.

Jeroen
+1, for extra security the class Student could be marked as internal, or be a private nested class of Students. This would prevent downcasting.
TheFogger
i though about the interfaces solution but like you said you can downcast to Student.but hiding Student is not good for me. I want to be able to use the constructor anywhere in the application.
Yaron Levi
+1  A: 

I personally like LukeH's answer quite a bit. The only issue is having to define the static StudentProperty class with static readonly delegate wrapper objects rather than your original eStudentProperty enum. Depending on your situation, this might not be easily usable by the caller.

The advantage of that code is that each StudentProperty<T> object is strongly typed according to its associated property, allowing the EnumerateBy method to return a strongly typed IEnumerable<T>.

Below is what I came up with. It's very similar to LukeH's answer in that I've provided a PropertyValues method similar to LukeH's EnumerateBy method, although my method returns an IEnumerable (non-generic). An issue with my implementation is that if you are iterating over a value-type property (such as Age), then there will be some boxing going on within the enumerator. However, if the consumer doesn't know enough about the property being iterated to call the Ages method that I've provided rather than PropertyValues(eStudentProperty.Age), then boxing will most likely occur in the caller's code regardless of whether I'm able to return an IEnumerable<int> or an IEnumerable. So I would argue that anyone who needs to call the PropertyValues method because they cannot call the Names, Cities, or Ages methods will not be able to avoid boxing with any implementation.

class Program
{
    static void Main(string[] args)
    {
        Students students = new Students();

        students.AddStudent(new Student { Age = 20, Name = "Stud1", City = "City1" });
        students.AddStudent(new Student { Age = 46, Name = "Stud2", City = "City2" });
        students.AddStudent(new Student { Age = 32, Name = "Stud3", City = "City3" });
        students.AddStudent(new Student { Age = 34, Name = "Stud4", City = "city4" });

        // in these two examples, you know exactly which property you want to iterate,
        // so call the corresponding iterator method directly.
        foreach (string studentCity in students.Cities())
        {
            Console.WriteLine(studentCity);
        }

        foreach (string studentName in students.Names())
        {
            Console.WriteLine(studentName);
        }

        // in these three cases, the DoSomethingWithPropertyValues method will not know which property is being iterated,
        // so it will have to use the PropertyValues method
        DoSomethingWithPropertyValues(students, eStudentProperty.Age);
        DoSomethingWithPropertyValues(students, eStudentProperty.Name);
        DoSomethingWithPropertyValues(students, eStudentProperty.City);
    }

    static void DoSomethingWithPropertyValues(Students students, eStudentProperty propertyToIterate)
    {
        // This method demonstrates use of the Students.PropertyValues method.
        // The property being iterated is determined by the propertyToIterate parameter,
        // therefore, this method cannot know which specific iterator method to call.
        // It will use the PropertyValues method instead.
        Console.WriteLine("Outputting values for the {0} property.", propertyToIterate);
        int index = 0;
        foreach (object value in students.PropertyValues(propertyToIterate))
        {
            Console.WriteLine("{0}: {1}", index++, value);
        }
    }
}

public class Students
{
    private List<Student> m_Students = new List<Student>();

    public void AddStudent(Student i_Student)
    {
        m_Students.Add(i_Student);
    }

    public IEnumerable PropertyValues(eStudentProperty property)
    {
        switch (property)
        {
            case eStudentProperty.Name:
                return this.Names();
            case eStudentProperty.City:
                return this.Cities();
            case eStudentProperty.Age:
                return this.Ages();
            default:
                throw new ArgumentOutOfRangeException("property");
        }
    }

    public IEnumerable<string> Names()
    {
        return m_Students.Select(s => s.Name);
    }

    public IEnumerable<string> Cities()
    {
        return m_Students.Select(s => s.City);
    }

    public IEnumerable<int> Ages()
    {
        return m_Students.Select(s => s.Age);
    }
}

public enum eStudentProperty
{
    Name,
    Age,
    City
}

public class Student
{
    public string Name { get; set; }

    public string City { get; set; }

    public int Age { get; set; }
}
Dr. Wily's Apprentice