HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaModerate

Generating a list of alternatively positioned odd and even number out of an array of unsorted integers

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
alternativelynumberevenarrayunsortedpositionedgeneratingandlistintegers

Problem

I had an half an hour interview with Microsoft for an intern position. I was asked to generate a list of alternatively positioned odd and even number out of an array of unsorted integers. I guess it was below the interviewer expectations since I did not get the offer. Can any one give me some hints on improving my code?

import java.util.ArrayList;

public class EvenOddArray {

private int min(int n, int m)
{
    int min;
    if (n > m){
        min = m;
    }
    else {
        min = n;
    }
    return min;
}

public ArrayList generateEvenOddArray(int[] list)
{
    ArrayList oddList = new ArrayList();
    ArrayList evenList = new ArrayList();
    ArrayList resultList = new ArrayList();

    for (int number : list) {
        if (number%2 == 0){
            evenList.add(number);
        }
        else {
            oddList.add(number);
        }

    }

    int numberOfEvenNumbers = evenList.size();
    int numberOfOddNumbers = oddList.size();
    int minOfOddAndEven = min(numberOfEvenNumbers,numberOfOddNumbers);
    ArrayList longestList = getLongestList(evenList,oddList);

    int i ;
    for (i = 0; i  getLongestList(ArrayList listone,
        ArrayList listTwo) {
    if (listone.size()listTwo.size()){
        return listone;
    }
    return null;
}

Solution

-
You probably should have used Math.min(). Even if it was requested by the interviewer to not use anything else besides ArrayList, min can be written much more concise as

return n <= m ? n : m


(1 line of code vs. 8)

-
This loop is little bit ugly:

for (i = 0; i < minOfOddAndEven; i++) {
    resultList.add(2*i,oddList.get(i) );
    resultList.add(2*i+1,evenList.get(i));          
}


While I think it should work (don't have a Java compiler handy to try) it's a bit confusing to add the numbers at specified indices. I think it is legal to call add(index, item) where index == size() which should add an element to the end of the list. So your code happens to work but you need to look twice to make sure. It's totally unnecessary to do this. You could have simply done:

for (i = 0; i < minOfOddAndEven; i++) {
    resultList.add(oddList.get(i));
    resultList.add(evenList.get(i));          
}


Same result but cleaner.

-
Copying the remainder of the list could have been done with subList:

if (longestList != null)
{
    resultList.addAll(longestList.subList(minOfOddAndEven, longestList.size());
}


-
Overall I think the interviewer might have wanted to see the use of iterators. You could have greatly simplified your code with this:

Iterator odds = oddList.iterator();
Iterator evens = evenList.iterator();

while (odds.hasNext() || evens.hasNext())
{
     if (odds.hasNext())
     {
         resultList.Add(odds.next());
     }
     if (evens.hasNext())
     {
         resultList.Add(evens.next());
     }
}


This way you iterate over both lists at once and don't have to implement the "which list is longer and then copy the remainder part"

Code Snippets

return n <= m ? n : m
for (i = 0; i < minOfOddAndEven; i++) {
    resultList.add(2*i,oddList.get(i) );
    resultList.add(2*i+1,evenList.get(i));          
}
for (i = 0; i < minOfOddAndEven; i++) {
    resultList.add(oddList.get(i));
    resultList.add(evenList.get(i));          
}
if (longestList != null)
{
    resultList.addAll(longestList.subList(minOfOddAndEven, longestList.size());
}
Iterator<Integer> odds = oddList.iterator();
Iterator<Integer> evens = evenList.iterator();

while (odds.hasNext() || evens.hasNext())
{
     if (odds.hasNext())
     {
         resultList.Add(odds.next());
     }
     if (evens.hasNext())
     {
         resultList.Add(evens.next());
     }
}

Context

StackExchange Code Review Q#35576, answer score: 10

Revisions (0)

No revisions yet.