Is there anything wrong with this Java code?

  • Java
  • Thread starter Jamin2112
  • Start date
  • Tags
    Code Java
  • #1
986
12
I had a phone interview with a guy and I thought it went well. At the end he told me to send him a Java program that reads from a file containing a list of integers, one per line, and prints out all the pairs that sum to 8. I thought I made a pretty good algorithm that finds all the pairs that sum to N. I thought I organized it and commented it well.

Anyways, he hasn't got back to me in a few days, so he must think it sucks or something.

Code:
import java.util.*;
import java.io.*;

public class SumToN {

         // N: The integer sum to which we want to search for 
         //     distinct integer pairs
	public static final int N = 8;
    
	public static void main(String[] args) throws FileNotFoundException {
	
		
		// "Numbers.txt" must be in the directory and must contain
		// on each line exactly 1 integer
		Scanner input = new Scanner(new File("Numbers.txt"));
		// Output will be saved in the directory as "Sum_Pairs.txt"
		PrintStream output = new PrintStream(new File("Sum_Pairs.txt"));
		
		/* Algorithm:
		
		   Create an instance of a binary tree which will act as 
	           a set to hold all the integers from the input. 
			
		   Then check, for each integer K in the input, whether K is 
		   already in the tree; if it's not, add it, but first output
		   K and (N - K) if (N - K) is in the tree.
			
		*/    
		Set<Integer> nums = new TreeSet<Integer>();
		while (input.hasNextInt()) {
		    int thisInt = input.nextInt();
		    if (!nums.contains(thisInt)) { 
			      if (nums.contains(N - thisInt)) 
					    output.printf("%d %d\n", thisInt, N - thisInt);
					nums.add(thisInt);   
			 }
		}
			
	}			
}
 

Answers and Replies

  • #2
Whatever editor or IDE you are using, turn off the tabs. If you have a good editor or IDE, you can enter tabs but it should immediately translate them to spaces. Look at what you posted. Does that look professional? Does it follow the Java coding standard? Be honest.

Always use braces, even for one-liners. This kind of coding will bite you one day.
Code:
if (nums.contains(N - thisInt)) 
    output.printf("%d %d\n", thisInt, N - thisInt);

What is to be done with duplicates? What if the user enters 1,1,1,7,7? Did you clarify the intent (requirements) with the interviewer?

Finally, your code is an O(n*log(n)) algorithm. That's not near as bad as an O(n2) algorithm, but java supplies a HashSet that turns this into an O(n) algorithm.
 
  • #3
Whatever editor or IDE you are using, turn off the tabs. If you have a good editor or IDE, you can enter tabs but it should immediately translate them to spaces. Look at what you posted. Does that look professional? Does it follow the Java coding standard? Be honest.

I have no idea what you mean by "Java coding standard." A lot of indentations did get screwed up when I copy-pasted my code.

Always use braces, even for one-liners. This kind of coding will bite you one day.
Code:
if (nums.contains(N - thisInt)) 
    output.printf("%d %d\n", thisInt, N - thisInt);

I thought it was considered elegant to use no braces on 1-line for, while and if statements.

What is to be done with duplicates? What if the user enters 1,1,1,7,7? Did you clarify the intent (requirements) with the interviewer?

No, these were the instructions he emailed me:

Write a simple java program. Given an array of integers stored in a file with one number per line, find all the pairs of integers that add up to 8. The output will also needs to be stored in a file with one pair per line.
An example input array looks like this:
3
2
4
6
5
7
20
1

The output will be like this:
3 5
2 6
7 1



Finally, your code is an O(n*log(n)) algorithm. That's not near as bad as an O(n2) algorithm, but java supplies a HashSet that turns this into an O(n) algorithm.

Yea, I was thinking about doing that.
 
  • #4
I have no idea what you mean by "Java coding standard."
Here's a link: http://www.oracle.com/technetwork/java/codeconv-138413.html [Broken]. It's been discussed in this forum, see [post]4496898[/post] and following.


A lot of indentations did get screwed up when I copy-pasted my code.
Exactly. The best way to ensure that indentation cannot get screwed up is to not have tabs in the source code. You can safely assume that people will use a monospaced font to look at your code, which means using spaces for whitespace is safe. The only thing you should assume about how tabs are handled is that they won't be handled how you use them. So either don't use them, or use an IDE/editor that translates tabs into spaces.


I thought it was considered elegant to use no braces on 1-line for, while and if statements.
In the 1970s when C was new? Yes. Nowadays? It's considered extremely bad form.


No, these were the instructions he emailed me:

Write a simple java program. Given an array of integers stored in a file with one number per line, find all the pairs of integers that add up to 8. The output will also needs to be stored in a file with one pair per line.
An example input array looks like this:
3
2
4
6
5
7
20
1

The output will be like this:
3 5
2 6
7 1
He wanted to see what assumptions you made and how you handled them. You made an assumption regarding duplicates but you did not document that assumption anywhere. When people write requirements they never spell everything out. Never. One of your jobs as a programmer is to document up front any assumptions you have made in translating those requirements into code. Hidden assumptions in code have been the cause of an immense number of problems and disagreements.

Where to spell them out? One good place is in the comments. It's good to place those assumptions up front rather than hidden deep within the code. In Java, that "up front" location would be in the javadoc commentary. You didn't use any javadoc style comments, which may be another reason that you didn't get a return call from the interviewer.
 
Last edited by a moderator:
  • #5
He wanted to see what assumptions you made and how you handled them. You made an assumption regarding duplicates but you did not document that assumption anywhere. When people write requirements they never spell everything out. Never.

When I emailed him back, I told him that I assumed they had to be unique pairs because his example had a "4" in the input but no "4 4" in the output. I also told him that, although his example showed only positive integers, I made no assumption that we were dealing with only positive integers.

I don't understand the point of vague instructions. I think I mentioned in this forum one of my interviews where all the guy asked was, "How do you find a word in a dictionary?" I didn't even know whether he was talking about a real-life dictionary or some data abstraction.
 
  • #6
I don't understand the point of vague instructions.
Neither do I, but they're more or less the only kind you ever get. Whether you handle that gracefully is one of the things they are testing...
 
Last edited:
  • #7
I don't understand the point of vague instructions.
A Michael Jackson song immediately came to mind:
If They Say -
Why, Why, Tell 'Em That Is Human Nature
Why, Why, Does He Do Me That Way
If They Say -
Why, Why, Tell 'Em That Is Human Nature
Why, Why, Does He Do Me That Way
 
  • #8
A Michael Jackson song immediately came to mind:
If They Say -
Why, Why, Tell 'Em That Is Human Nature
Why, Why, Does He Do Me That Way
If They Say -
Why, Why, Tell 'Em That Is Human Nature
Why, Why, Does He Do Me That Way

Well, the guy just invited me to an onsite interview. Time for a massive Java review session.
 
  • #10
I don't understand the point of vague instructions. I think I mentioned in this forum one of my interviews where all the guy asked was, "How do you find a word in a dictionary?" I didn't even know whether he was talking about a real-life dictionary or some data abstraction.
If you aren't clear what he means, ask for clarification. If you don't get clarification, make sure that you state what your assumptions are. One of the things that interviewers often look for is how well you deal with ambiguity.
 
  • #11
If you aren't clear what he means, ask for clarification. If you don't get clarification, make sure that you state what your assumptions are. One of the things that interviewers often look for is how well you deal with ambiguity.

Sounds good.

If I need to think about a programming question, should I tell them "I need a moment to think" and then mull it over silently until I have a clear answer, or should I always launch right into a think-out-loud, stream-of-consciousness answer that evolves? I feel like I did the latter in my last interview and it didn't go well as a result.
 
  • #12
"I need a moment to think" definitely seems better to me.
 

Suggested for: Is there anything wrong with this Java code?

Replies
12
Views
755
Replies
8
Views
591
Replies
5
Views
902
Replies
1
Views
298
Replies
7
Views
1K
2
Replies
60
Views
638
Replies
2
Views
746
Replies
11
Views
549
Back
Top