views:

130

answers:

6

I have two student objects.

class Student{
int physics;
int english;
int chemistry;
}

I need to compare Student A marks in each subject with Student B's marks in all the subjects.

A marks in physics needs to be compared with B's marks in physics, english, chemistry. Similarly A's english with all the three of B.

if there is atleast one match say A's chemistry marks are equal to B's english marks, then stop execution and return false.

My logic is

if(a.getPhysics==b.getPhysics || a.getPhysics==b.getEnglish || a.phy==b.chem ||  ...){
   return false;
 }

Is this better or any other good logic ??????

+1  A: 

A little improvment would be to create a method in the Student class to do it.

Michael Pereira
+1  A: 

Well, you will have to make O(n^2) comparisons in any case, the question is how clean the code will be.

What you are suggesting now is good for 6 boolean comparisons, what if you have 30 subjects? Will you maintain the hundredss of comparisons you need to make?

Keeping it simple, I would prefer keeping the grades in a List or a Map and then do a nested iteration:

for (int gradeA : thisStudent.getGrades()) {
    for (int gradeB : otherStudent.getGrades()) {
        if (gradeA == gradeB) return false;
    }
}
return true;

Of course this code needs to be adapted to your scenario (different iteration on List vs. Map, optimization by not checking each grade every time, extracting a method out of this, etc...)

Yuval A
+1  A: 

Those attributes (the courses physics, english, ...) shouldn't be in the Student class. A better option would be to create a CourseModel where your store all your Courses and keep track of all the Students who are enrolled in a course. From your CourseModel, you can query for a particular Student and get all their courses back (as a array/collection). When you have two collections/arrays, simply create a nested for-statement to compare all of them.

Bart Kiers
+1  A: 

Use HashSets:

Set<Integer> aMarks = new HashSet<Integer>();
Set<Integer> bMarks = new HashSet<Integer>();

Collections.addAll(aMarks, 2, 3, 9);
Collections.addAll(bMarks, 4, 2, 2);

boolean check = Collections.disjoint(aMarks, bMarks);
return check;

The values are just for test. You can change Collections.addAll(...) with a new method Student.getMarksAsSet()

True Soft
You will need to save the grades in the set in addition to another data structure since you lose granularity over which grade is for which subject.
Yuval A
But unfortunately, I can not change the student object. It's in a jar.
abhishek
If you can't change the Student class, add your marks in the set one by one: `aMarks.add(a.getPhysics)`, `aMarks.add(a.getEnglish)` ...
True Soft
If the `Student` class is in a jar, extend it and add other methods to the new one.
True Soft
Instead of extending the unmodifiable `Student` class, you could also wrap it (create `MyStudent` that has a `Student` field and delegates to it when necessary). One obvious thing you would be able to do then is to add your own methods. Also, you would restrict the interface to just the methods you need from `Student`.
Lyudmil
+1  A: 

You could add the ability for Student to return its marks as a set:

public class Student {
   private int physics;
   private int english;
   private int chemistry;

   public Student(int physics, int english, int chemistry) {
      this.physics = physics;
      this.english = english;
      this.chemistry = chemistry;
   }

   public Set<Integer> marks() {
      return new HashSet<Integer>(Arrays.asList(physics, english, chemistry));
   }
}

Then, when are trying to determine if two students match, all you need to see is whether their two respective sets of marks are disjoint, as StudentMatcher does:

public class StudentMatcher {
   public boolean matches(Student student1, Student student2) {
      Set<Integer> studentMarks1 = student1.marks();
      Set<Integer> studentMarks2 = student2.marks();
      return haveIntersection(studentMarks1, studentMarks2);
   }

   private boolean haveIntersection(Set<Integer> studentMarks1, Set<Integer> studentMarks2) {
      return studentMarks1.removeAll(studentMarks2);
   }
}

And here is a unit test to verify it works:

public class StudentMatcherTest {
   @Test
   public void matches() {
      StudentMatcher matcher = new StudentMatcher();
      Student student1 = new Student(34, 45, 66);
      Student student2 = new Student(99, 55, 34);
      Student student3 = new Student(11, 22, 33);

      assertTrue("Should match", matcher.matches(student1, student2));
      assertFalse("Should not match", matcher.matches(student1, student3));
   }
}

There is more that can be done to make this better, but I'm assuming your code is more complicated than what you've posted, so hopefully this is enough to get you on a better path.

Lyudmil
A: 

If the marks are in a small range (A-F rather than percent), and you need to compare marks in many subjects rather than the three you give, populate an array of boolean values to hold whether the first student has the given mark, then for the second check whether the array has a value set. That would be O(N+M) where N is the number of subjects and M the number of possible grades.

If you only have the three subjects, having the tests hard coded isn't that bad - you'll need six lines to get the marks from each anyway.

Pete Kirkham