Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.java.programmer > #38824 > unrolled thread

Interview question - better solution for MergePurgeAndReverseOrder()

Back to article view | Back to comp.lang.java.programmer

Contents

  Interview question - better solution for MergePurgeAndReverseOrder() Graeme Geldenhuys <graemeg@example.net> - 2019-03-29 15:01 +0000
    Re: Interview question - better solution for MergePurgeAndReverseOrder() Eric Sosman <esosman@comcast-dot-net.invalid> - 2019-03-29 11:19 -0400
      Re: Interview question - better solution for MergePurgeAndReverseOrder() Graeme Geldenhuys <graemeg@example.net> - 2019-03-29 16:42 +0000
        Re: Interview question - better solution for MergePurgeAndReverseOrder() Eric Sosman <esosman@comcast-dot-net.invalid> - 2019-03-29 13:47 -0400
          Re: Interview question - better solution for MergePurgeAndReverseOrder() Eric Sosman <esosman@comcast-dot-net.invalid> - 2019-03-29 13:53 -0400
          Re: Interview question - better solution for MergePurgeAndReverseOrder() Daniele Futtorovic <da.futt.news@laposte-dot-net.invalid> - 2019-03-29 19:42 +0100
    Re: Interview question - better solution for MergePurgeAndReverseOrder() Graeme Geldenhuys <graemeg@example.net> - 2019-03-29 16:53 +0000
      Re: Interview question - better solution for MergePurgeAndReverseOrder() Eric Douglas <e.d.programmer@gmail.com> - 2019-03-29 10:19 -0700
      Re: Interview question - better solution for MergePurgeAndReverseOrder() Jukka Lahtinen <jtfjdehf@hotmail.com.invalid> - 2019-03-30 11:23 +0200
        Re: Interview question - better solution for MergePurgeAndReverseOrder() Graeme Geldenhuys <graemeg@example.net> - 2019-03-30 11:16 +0000
    Re: Interview question - better solution for MergePurgeAndReverseOrder() Eric Douglas <e.d.programmer@gmail.com> - 2019-03-29 13:20 -0700
      Re: Interview question - better solution for MergePurgeAndReverseOrder() Graeme Geldenhuys <graemeg@example.net> - 2019-03-30 11:21 +0000
        Re: Interview question - better solution for MergePurgeAndReverseOrder() Eric Douglas <e.d.programmer@gmail.com> - 2019-04-01 04:56 -0700
    Re: Interview question - better solution for MergePurgeAndReverseOrder() bursejan@gmail.com - 2019-03-29 15:40 -0700

#38824 — Interview question - better solution for MergePurgeAndReverseOrder()

Hi,

I recently attended a Java developer interview and had to implement the
MergePurgeAndReverseSort() method. I managed to complete it and
afterwards wrote some unit tests to confirm that it works. But is there
a better way to implement it, or did I do an okay job about it.

I can post the unit tests too if that might help.

Any feedback would be very much appreciated.

Regards,
  Graeme



***********************************************
public class Main {

	public static void main(String[] args) {
		Integer[] v1 = new Integer[] { 1, 2, null, 3 };
		Integer[] v2 = new Integer[] { 1, 2, 3, 4 };
		int[] arr = Util.MergePurgeAndReverseSort(v1, v2, true);

		for (int i = 0; i < arr.length; i++)
			System.out.println(arr[i]);

	}

}

***********************************************

import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;

public class Util {

	/*
	 * [x] Merge the two arrays
	 * [x] remove duplicates
	 * [x] remove NULL's
	 * [x] sort in reverse order
	 * [x] return as an int[] array
	 */
	public static int[] MergePurgeAndReverseSort(Integer[] values1,
Integer[] values2, boolean reverse) {
		// merge arrays
		Integer[] v1andv2 = new Integer[values1.length + values2.length];
		System.arraycopy(values1, 0, v1andv2, 0, values1.length);
		System.arraycopy(values2, 0, v1andv2, values1.length, values2.length);

		// Use a set to remove duplicates
		Set<Integer> intSet = new HashSet<>();
		for (Integer item : v1andv2) {
			// We could check for null's here too, or handle it separately as
shown below
			if (item != null)
				intSet.add(item);
		}

		// sort in reverse order
		List<Integer> lst;
		if (reverse)
			lst =
intSet.stream().sorted(Collections.reverseOrder()).collect(Collectors.toList());
		else
			lst = intSet.stream().sorted().collect(Collectors.toList());


		// unbox data
		int[] n = lst.stream().mapToInt(Integer::intValue).toArray();

		return n;
	}
}
	
***********************************************

[toc] | [next] | [standalone]

#38825

On 3/29/2019 11:01 AM, Graeme Geldenhuys wrote:
> Hi,
> 
> I recently attended a Java developer interview and had to implement the
> MergePurgeAndReverseSort() method. I managed to complete it and
> afterwards wrote some unit tests to confirm that it works. But is there
> a better way to implement it, or did I do an okay job about it.
> 
> I can post the unit tests too if that might help.
> 
> Any feedback would be very much appreciated.
> 
> [Code snipped; see up-thread]

     Looks pretty much okay, although it could be tightened a bit.
Most of the opportunities for tightening come down to matters of
taste, but one that stands out (to me, anyway) is the matter of
moving all the Integers into a single array merely to move them
again into a Set -- Why not just pop 'em into the Set right away?

     I personally would have written the rest somewhat differently
than you did, but the differences don't seem important.  Matters
of taste, as I wrote earlier.

-- 
esosman@comcast-dot-net.invalid
Six hundred sixty-three days to go.

[toc] | [prev] | [next] | [standalone]

#38827

On 29/03/2019 15:19, Eric Sosman wrote:
> I personally would have written the rest somewhat differently
> than you did, but the differences don't seem important.  Matters
> of taste, as I wrote earlier.

Thank you for your feedback.

Only if you have the time, could you show me how you would have
implemented that method. I like to educate myself further in different
ways of doing things (Java's libraries are daunting to learn, and I'm
bound to miss out on some parts). I'm trying to get to grips on all the
ways of using the various Collection classes in Java.

Regards,
  Graeme

[toc] | [prev] | [next] | [standalone]

#38830

On 3/29/2019 12:42 PM, Graeme Geldenhuys wrote:
> On 29/03/2019 15:19, Eric Sosman wrote:
>> I personally would have written the rest somewhat differently
>> than you did, but the differences don't seem important.  Matters
>> of taste, as I wrote earlier.
> 
> Thank you for your feedback.
> 
> Only if you have the time, could you show me how you would have
> implemented that method. I like to educate myself further in different
> ways of doing things (Java's libraries are daunting to learn, and I'm
> bound to miss out on some parts). I'm trying to get to grips on all the
> ways of using the various Collection classes in Java.

     Here's one way (just typed in, not tested):

    public static int[] MergePurgeAndReverseSort_1(
          Integer[] values1, Integer[] values2, boolean reverse) {
       Set<Integer> allValues = new HashSet<>();
       for (Integer value : values1) {
          if (value != null) {
             allValues.add(value);
          }
       }
       for (Integer value : values2) {
          if (value != null) {
             allValues.add(value);
          }
       }
       Comparator<Integer> compare = reverse
             ? Collections.reverseOrder() : Integer::compare;
       return allValues.stream()
             .sorted(compare)
             .mapToInt(Integer::intValue)
             .toArray();
    }

This has a nagging infelicity: The null-test-and-add must be written
twice.  Whenever "the same thing" is written more than once there's a
chance that the appearances might disagree slightly (imagine a more
complex test and a maintenance programmer working hastily), so it's
almost always better to write such things once only.  One way would
be to write two unconditional adds and follow the two loops with a
single allValues.remove(null) to purge any undesirables; another would
be to make all the values flow through a single test-and-add:

    public static int[] MergePurgeAndReverseSort_2(
          Integer[] values1, Integer[] values2, boolean reverse) {
       Set<Integer> allValues = Stream.concat(
                Arrays.asList(values1).stream(),
                Arrays.asList(values2).stream())
             .filter((value) -> value != null)
             .collect(Collectors.toSet());
       Comparator<Integer> compare = reverse
             ? Collections.reverseOrder() : Integer::compare;
       return allValues.stream()
             .sorted(compare)
             .mapToInt(Integer::intValue)
             .toArray();
    }

Having come this far, and following the principle that one might as
well be hanged for a sheep as for a lamb, we arrive at:

    public static int[] MergePurgeAndReverseSort_3(
          Integer[] values1, Integer[] values2, boolean reverse) {
       return Stream.concat(
                Arrays.asList(values1).stream(),
                Arrays.asList(values2).stream())
             .filter((value) -> value != null)
             .distinct()
             .sorted(reverse ? Collections.reverseOrder() : 
Integer::compare)
             .mapToInt(Integer::intValue)
             .toArray();
    }

... and proudly announce "It's a one-liner!"  (The APL fans, of course,
sneer at us.)

     So: No big deal, just lots of ways to acquire cats' skins.

-- 
esosman@comcast-dot-net.invalid
Six hundred sixty-three days to go.

[toc] | [prev] | [next] | [standalone]

#38831

On 3/29/2019 1:47 PM, Eric Sosman wrote:
> [...]
>     public static int[] MergePurgeAndReverseSort_2(
>           Integer[] values1, Integer[] values2, boolean reverse) {
>        Set<Integer> allValues = Stream.concat(
>                 Arrays.asList(values1).stream(),
>                 Arrays.asList(values2).stream())
>  [...]

     Oh, ratzafratz!  There's no need for the asList() calls; I
could simply have used

	Stream.concat(Arrays.stream(values1), Arrays.stream(values2))

and avoided all that silly running around.  Sorry about that, Chief.

-- 
esosman@comcast-dot-net.invalid
Six hundred sixty-three days to go.

[toc] | [prev] | [next] | [standalone]

#38833

On 2019-03-29 18:47, Eric Sosman wrote:
> 
> Having come this far, and following the principle that one might as
> well be hanged for a sheep as for a lamb, we arrive at:
> 
>    public static int[] MergePurgeAndReverseSort_3(
>          Integer[] values1, Integer[] values2, boolean reverse) {
>       return Stream.concat(
>                Arrays.asList(values1).stream(),
>                Arrays.asList(values2).stream())
>             .filter((value) -> value != null)
>             .distinct()
>             .sorted(reverse ? Collections.reverseOrder() :
> Integer::compare)
>             .mapToInt(Integer::intValue)
>             .toArray();
>    }
> 
> ... and proudly announce "It's a one-liner!"  (The APL fans, of course,
> sneer at us.)
> 
>     So: No big deal, just lots of ways to acquire cats' skins.

While we're on the matter of taste, I would also have used:
> .filter(Objects::nonNull)
instead of
> .filter((value) -> value != null)

-- 
DF.

[toc] | [prev] | [next] | [standalone]

#38828

On 29/03/2019 15:30, Stefan Ram wrote:
>   First observation: "And" in a method name is a indication that
>   the method does too many things or has no clear responsibility.
> 
>   So one should check whether it is possible to refactor, i.e.,
>   to split the method or improve the name.

I fully agree, and that is how I would implement code "in the real
world". But as this was an interview technical test, I was given two
classes and the second one had an empty implementation for
MergePurgeAndReverseSort().

I was not allowed to change anything, other than implement the method.

>   Next observation: "implement it". Implement what?
>   Where's the specification? They should have given
>   you a specification of what the method is supposed
>   to do. May be this is what follows in the comment?
> 
>>	/*
>>	 * [x] Merge the two arrays
>>	 * [x] remove duplicates
>>	 * [x] remove NULL's
>>	 * [x] sort in reverse order
>>	 * [x] return as an int[] array
>>	 */

It was a while back. I can't remember if they supplied that comment, or
if I did it to make sure I implemented everything that was asked. Either
way, that checklist is what the method had to do in the end.


Regards,
  Graeme

[toc] | [prev] | [next] | [standalone]

#38829

On Friday, March 29, 2019 at 12:53:18 PM UTC-4, Graeme Geldenhuys wrote:
> I fully agree, and that is how I would implement code "in the real
> world". But as this was an interview technical test, I was given two
> classes and the second one had an empty implementation for
> MergePurgeAndReverseSort().
> 
> I was not allowed to change anything, other than implement the method.
> 
It sounds like they were trying to give you a hint that you shouldn't want to work there.  Normalizing code is the best answer.  That sounds like 3 separate methods.

[toc] | [prev] | [next] | [standalone]

#38836

Graeme Geldenhuys <graemeg@example.net> writes:
> On 29/03/2019 15:30, Stefan Ram wrote:

>>   So one should check whether it is possible to refactor, i.e.,
>>   to split the method or improve the name.

> I was not allowed to change anything, other than implement the method.

You could have implemented it to call the methods resulting from the
split.
Besides, method names should start with a lowercase letter..

-- 
Jukka Lahtinen

[toc] | [prev] | [next] | [standalone]

#38838

On 30/03/2019 09:23, Jukka Lahtinen wrote:
> You could have implemented it to call the methods resulting from the
> split.

Again, I couldn't do that. Lots of technical interview tests are done
online these days and they have a similar format. If you look at
HackerRank.com you will see how it works. The whole program is supplied
(static text you can't change) except for the area you need to implement
(the body of the method in question).

But again, I definitely agree with all the points made. The method name
is too long, the method does more than one thing etc. But it seems good
design was not what they were testing, but rather the knowledge of how
to use the java language and libraries.

Regards,
  Graeme

[toc] | [prev] | [next] | [standalone]

#38834

On Friday, March 29, 2019 at 11:01:13 AM UTC-4, Graeme Geldenhuys wrote:
> Hi,
> 
> I recently attended a Java developer interview and had to implement the
> MergePurgeAndReverseSort() method. I managed to complete it and
> afterwards wrote some unit tests to confirm that it works. But is there
> a better way to implement it, or did I do an okay job about it.
> 
> I can post the unit tests too if that might help.
> 
> Any feedback would be very much appreciated.
> 
> Regards,
>   Graeme
> 
> 
Define better.  Are you concerned with appearance or performance?
I'd be thrilled to find a candidate who can write normalized readable code that produces correct output.
Consider redundancy and re-usability.  Could the method you're writing be used elsewhere?  Are you writing the same code in 2 places, where both will need to be updated if it ever changes?

Whether performance matters depends on your volume of data.  Shaving off nanoseconds might matter when you're executing it a billion times, though hardware has also gotten cheaper.  The problems we had getting reports to finish in an 8 hour nightly window 20 years ago shouldn't be an issue today.  These days it seems to be more about the user experience and impatience, can they wait 1 second instead of 5.

I'd envy the company that can afford to find the programmer that can write perfect code from the start.  Most should be happy if you know the basics and can learn and adapt quickly.  I wonder how closely your test relates to the code you'd actually be working on.  At my job we tend to hire kids fresh out of college, I just give them a fizzbuzz test.  Actual coding involves "what is this data?" ie it's a corporation which owns multiple companies, you're processing gift certificate redemptions, can you match them to the issuance?

[toc] | [prev] | [next] | [standalone]

#38839

On 29/03/2019 20:20, Eric Douglas wrote:
> Define better.  Are you concerned with appearance or performance?

I guess that was an "up in the air" question indeed. I haven't use Java
in my last few jobs (10+ years), but did use Java before that. So my
knowledge is mostly on old syntax and libraries. I'm trying to get up to
speed with "the new way of doing things", even though the old way will
most likely still work.

Regards,
  Graeme

[toc] | [prev] | [next] | [standalone]

#38846

On Saturday, March 30, 2019 at 7:21:26 AM UTC-4, Graeme Geldenhuys wrote:
> On 29/03/2019 20:20, Eric Douglas wrote:
> > Define better.  Are you concerned with appearance or performance?
> 
> I guess that was an "up in the air" question indeed. I haven't use Java
> in my last few jobs (10+ years), but did use Java before that. So my
> knowledge is mostly on old syntax and libraries. I'm trying to get up to
> speed with "the new way of doing things", even though the old way will
> most likely still work.
> 
> Regards,
>   Graeme

There is the question of "show your work" which says write it out in a way any Java coder can follow (format and variable names may be important) versus the question of "are you following the changes to the language" which suggests coding it using the latest lambda syntax.  I suspect a lot of companies are not trying to use the latest syntax.  If they're even running Java 8 (some might still be on 6) they may only use new syntax in certain new development, or if it offers significant performance improvement.

[toc] | [prev] | [next] | [standalone]

#38835

The word "implement" got a total new meaning. 

Did they notice that you didn't implement 
anything, only used some library calls in the
most complicated way. 

Basically if the two input vectors are already 
sorted, you would possibly implement something 
totally different. Namely a pairing algorithm, 

which reads as follows:

   public static int[] merge(int[] a, int[] b) {
      int[] res=new int[a.length+b.length];
      int i=0;
      int j=0;
      int k=0;
      while (j<a.length && k<b.length) {
         if (a[j]<b[k]) {
            res[i++]=a[j++];
         } else {
            res[i++]=b[k++];
         }
      }
      while (j<a.length)
            res[i++]=a[j++];
      while (k<b.length)
            res[i++]=b[k++];
      return res;
   }

For simplicity, to make the point, the above is without
reverse/duplicate/null. The name "merge" tells me that 
this was asked. Otherwise it would be "union" and 

not "merge". Here is an example run. 

    public static void main(String[] args) {
        int[] a = new int[]{1, 2, 3};
        int[] b = new int[]{1, 2, 3, 4};
        int[] res = merge(a, b);
        for (int i = 0; i < res.length; i++)
            System.out.println("res[" + i + "]=" + res[i]);
    }

Thats the output:

res[0]=1
res[1]=1
res[2]=2
res[3]=2
res[4]=3
res[5]=3
res[6]=4

On Friday, March 29, 2019 at 4:01:13 PM UTC+1, Graeme Geldenhuys wrote:
> Hi,
> 
> I recently attended a Java developer interview and had to implement the
> MergePurgeAndReverseSort() method. I managed to complete it and
> afterwards wrote some unit tests to confirm that it works. But is there
> a better way to implement it, or did I do an okay job about it.
> 
> I can post the unit tests too if that might help.
> 
> Any feedback would be very much appreciated.
> 
> Regards,
>   Graeme
> 
> 
> 
> ***********************************************
> public class Main {
> 
> 	public static void main(String[] args) {
> 		Integer[] v1 = new Integer[] { 1, 2, null, 3 };
> 		Integer[] v2 = new Integer[] { 1, 2, 3, 4 };
> 		int[] arr = Util.MergePurgeAndReverseSort(v1, v2, true);
> 
> 		for (int i = 0; i < arr.length; i++)
> 			System.out.println(arr[i]);
> 
> 	}
> 
> }
> 
> ***********************************************
> 
> import java.util.Collections;
> import java.util.HashSet;
> import java.util.List;
> import java.util.Set;
> import java.util.stream.Collectors;
> 
> public class Util {
> 
> 	/*
> 	 * [x] Merge the two arrays
> 	 * [x] remove duplicates
> 	 * [x] remove NULL's
> 	 * [x] sort in reverse order
> 	 * [x] return as an int[] array
> 	 */
> 	public static int[] MergePurgeAndReverseSort(Integer[] values1,
> Integer[] values2, boolean reverse) {
> 		// merge arrays
> 		Integer[] v1andv2 = new Integer[values1.length + values2.length];
> 		System.arraycopy(values1, 0, v1andv2, 0, values1.length);
> 		System.arraycopy(values2, 0, v1andv2, values1.length, values2.length);
> 
> 		// Use a set to remove duplicates
> 		Set<Integer> intSet = new HashSet<>();
> 		for (Integer item : v1andv2) {
> 			// We could check for null's here too, or handle it separately as
> shown below
> 			if (item != null)
> 				intSet.add(item);
> 		}
> 
> 		// sort in reverse order
> 		List<Integer> lst;
> 		if (reverse)
> 			lst =
> intSet.stream().sorted(Collections.reverseOrder()).collect(Collectors.toList());
> 		else
> 			lst = intSet.stream().sorted().collect(Collectors.toList());
> 
> 
> 		// unbox data
> 		int[] n = lst.stream().mapToInt(Integer::intValue).toArray();
> 
> 		return n;
> 	}
> }
> 	
> ***********************************************

[toc] | [prev] | [standalone]

Back to top | Article view | comp.lang.java.programmer

csiph-web