• Post Reply Bookmark Topic Watch Topic
  • New Topic

Feeling unjustifiably rejected! Failed in code test  RSS feed

 
Ranch Hand
Posts: 59
1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Recently a company wanted me to write code for the following scenario, as part of their routine code test, before being considered for formal interview:

When given a list of numbers, the program should read them, and return top 20% (of the entire list which could vary each time) for grade A, then the next 20% for grade B, next 20% for grade C, next 20% for grade D, and the last 20% (and perhaps anything below) for the last grade E.

I wrote the following code and they told me it is wrong and failed me. I feel it is injustice, my program does exactly what they wanted. Can you spot any failing reason in my code? They never gave me a reason why they think my code is incorrect.

 
Marshal
Posts: 59437
187
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Don't write such a long main() method. The ideal length of a main method is one statement.
Why are you declaring the Lists as ArrayList not List?
Why are you using a BigDecimal and then rounding the result to a whole number?
The comments in lines 23 and 29 are not at all useful.
If you have been told the bottom 20% get an E, why is there a List for grade F?
There are spelling errors in some of the variable names. It should be totalScoreCount, not totlScoreCount. Maybe that is different from what you actually wrote for your interview. It is not clear what mkpList means.
If you are using a Scanner, why not use its methods rather than splitting the String and using Double.parseDouble()? And then you are using a convoluted method to convert that to a BigDecimal.
If the number is out of range, why are you throwing a plain simple Exception? And is it a good idea to throw and catch the same Exception in the same method?
Please explain how the combination of a loop and the switch work. Why are you trying to empty the List? I think that is unnecessary. There appears to be the same code for each of the cases. It would be better to work out some way to avoid repeated code. There is also repeated code after line 111. That looks like something which belongs in a separate method.
I would prefer a proper object‑oriented approach, with no method being labelled static. I would have made the List an instance field. I wouldn't have used float arithmetic anywhere because of its imprecision (line 129).
You can see in that method that there is inconsistent formatting; sometimes there are spaces around operators and sometimes not. That makes the code hard to read. Lines like No 135 are also hard to read because of their length.
It is not clear what the nested loops and if‑elses in lines 154‑175 mean. It is also not clear why you are removing things from the List just before that block. Is there no simpler way to remove elements from a List than to use a loop?
It isn' a good idea to call a method with void type getXXX() (line 181). What does that method do that is different from System.out.println(myList);?
 
Marshal
Posts: 4425
283
Clojure IntelliJ IDE Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Well it does compile and run, although declaring a package name adds the complication of having to create directories for the CalcGrades.java file to live in. You've lost points on the "make it as easy as possible" test already.

- Your code categorises the scores as grades A through F but the requirements only say A through E.
- You constrain the input values to numbers between 0 and 100, but the requirements dictate no such restriction.
- Why are you rounding to integers? No mention of that in the requirements.
- Your code mentions students, but students are not mentioned in the requirements.
- You have a getXXX() method with a void return. Typically a get method gets and returns something.
- I expect some sorting to happen at some point, but only once. You have done it twice which is computationally expensive and unnecessary.

These are the first things that really jump out at me, along with the nagging feeling that there's way way way too much code here. My gut feeling says that a tidy solution would not be as convoluted as this.

I've spent about 5 minutes on this, so don't think this feedback is a comprehensive code review. Just my first impressions.
 
Ravi Desigan
Ranch Hand
Posts: 59
1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Ritchie, Tim,

My bad! The specification mentions Grades A through F. That's why the switch/case has 6 conditions. This is the actual specification, I will post why I have coded all that I have in the subsequent post for clarity.

Specification:
=========
A teacher gives a class of students an exam. He decides to grade the
exam using the following method:
* A score in the top 20% of all scores is an A.
* A score in the next 20% of scores is a B.
* A score in the next 20% of scores is a C.
* A score in the next 20% of scores is a D.
* A score in the bottom 20% of scores is an F.
For example, if a class of 20 students has the following scores:
99, 92, 91, 91, 89, 85, 83, 82, 80, 79, 78, 78, 77, 76, 75, 74, 62, 55, 43, 20
The first four scores (99, 92, 91, 91) would be an A, scores 5 through 8 (89,
85, 83, 82) would be a B, scores 9 through 12 (80, 79, 78, 78) would be a C,
scores 13 through 16 would be a D (77, 76, 75, 74) and scores 17 through 20
(62, 55, 43, 20) would be an F.
Write a function that takes an arbitrary (possibly unsorted) score list of any
length (not necessarily the list used as an example above) as a parameter, and
returns a grade for each score.
Page 1
Athenium-take-home-interview-exercise.txt
Note that since the list of scores can be of any length the number of scores
may not necessarily be divisible by 5. Please make sure your function will
handle those cases gracefully and consistently.
ADDITIONAL REQUIREMENT: If there are two (or more) scores that are identical,
then those identical numerical scores must always receive the same grade, even
if that causes some grades to contain more scores than others.
 
Ravi Desigan
Ranch Hand
Posts: 59
1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
As you can see, the fetchNperformersFromMasterList method does the core functionality.

The main method invokes the fetchNperformersFromMasterList method 6 times - to populate Grade A through Grade F - totally 6 arraylists. Therefore main method performs switch/case routine to loop through exactly 6 times.

When the fetchNperformersFromMasterList method is called each time, it takes a master list, takes the number of students that should be in for the 20% criterion, and subtracts that many number of students from the topmost student backwards, and returns a sublist of these students.

The main method, goes from Grade A to F, and removes each time, the sublist from the main list before passing on the main list to the fetch students for the next lower grade.

The fetchNperformersFromMasterList method does the following:

1. Return a sublist from the mainlist.
2. Remove students populated in the sublist from the mainlist.
3. Take a peek into the next lower grade, look for a duplicate grade in which case, that number should also belong to this grade, and if duplicates are found, add them to the current list and remove them from the next lower grade.

What I want to know, is that, although my programming style may not be perfect, atleast my logic isn't incorrect. The company seemed to suggest my solution itself is wrong, totally.
 
Tim Cooke
Marshal
Posts: 4425
283
Clojure IntelliJ IDE Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
They do have a point.

Answer me this: To split up a list of items into separate buckets where each bucket contains 20% of the items in the original list, how many buckets would you require?

Then answer me this: How many buckets (grades) does your code have?

Have you spotted where you went wrong yet?
 
Campbell Ritchie
Marshal
Posts: 59437
187
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Ravi Desigan wrote:. . . . The specification mentions Grades A through F.

Not in what you quoted; there is no E.

. . . If there are two (or more) scores that are identical, then those identical numerical scores must always receive the same grade, . . . .

Where have you implemented that part?
 
Campbell Ritchie
Marshal
Posts: 59437
187
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
As I implied yesterday, getting the “right” answer doesn't mean your code is correct. Style of coding is important, too, which is why I mentioned spellings. Also, they would be worried about the long methods. That would make them think you are going to write applications which will be difficult to maintain and upgrade after you leave the company.
 
Marshal
Posts: 5804
401
BSD
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
1. After you get all code working, the other problem might be that you haven't demonstrated any written tests for your code. Have you got them? If no, you can not state your code works and gives correct answer.

2.

Ravi Desigan wrote:fetchNperformersFromMasterList method does the core functionality


instructions wrote:Write a function that takes an arbitrary (possibly unsorted) score list...


You don't have any method which would take List as an argument. For some reason you assumed list must be an ArrayList. Which is not what is specified. Plus you added some extra parameter.

Some of the things you need to take from this interview are:
  • Need to do exactly what is asked, no more - failed on that
  • Code must be tested, proof provided - failed on that
  • Code must be readable - failed on that
  • Code must be well formatted, formatting consistent - failed on that

  • This code is over-complicated. You were asked to write a function/method, one in total, which is accessible to the wide world, meaning one public function and the rest methods private which would serve as helper methods.

    3. Less than optimum outcome is also an outcome - so try to take as much as you can from it. I assume it was home interview exercise, meaning you had fair amount of time, so you had to come up with much more solid solution and code in general.
     
    Liutauras Vilda
    Marshal
    Posts: 5804
    401
    BSD
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    But hey. You were invited to an interview - meaning your resume is attracting, so you'll get another interview and hopefully you'll do better then Good Luck!
     
    Ravi Desigan
    Ranch Hand
    Posts: 59
    1
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    I don't know what to say, I did attach a printout of sample run/test results and send it to them along with the code!

    Please see the result from the program:

    1.
    run:
    Please enter test scores that range from 0 to 100 in a row
    Separate each grade with a White space.
    INFO: Scores with decimals will be rounded to the closest whole number.

    99 92 91 91 89 85 83 82 80 79 78 78 77 76 75 74 62 55 43 20
    Total count of scores entered is: 20 , approx. number of scores in each range is: 4
    [ 20 43 55 62 74 75 76 77 78 78 79 80 82 83 85 89 91 91 92 99 ]
    Following is the break-up of the input scores:
    **********************************************
    Scores of Grade A performers are:
    [ 91 91 92 99 ]
    Scores of Grade B performers are:
    [ 82 83 85 89 ]
    Scores of Grade C performers are:
    [ 78 78 79 80 ]
    Scores of Grade D performers are:
    [ 74 75 76 77 ]
    Scores of Grade E performers are:
    [ 20 43 55 62 ]
    Scores of Grade F performers are:
    [ ]
    BUILD SUCCESSFUL (total time: 10 seconds)






    2. 87 45.8 23.9 67 76 54 45 77.88 65.4 33 12.9 39 55.46798 29.888 73.6 45 54 23.9 87 86 76.5 70.5 69.5 69.4
    run:
    Please enter test scores that range from 0 to 100 in a row
    Separate each grade with a White space.
    INFO: Scores with decimals will be rounded to the closest whole number.

    87 45.8 23.9 67 76 54 45 77.88 65.4 33 12.9 39 55.46798 29.888 73.6 45 54 23.9 87 86 76.5
    Total count of scores entered is: 21 , approx. number of scores in each range is: 4
    [ 13 24 24 30 33 39 45 45 46 54 54 55 65 67 74 76 77 78 86 87 87 ]
    Following is the break-up of the input scores:
    **********************************************
    Scores of Grade A performers are:
    [ 78 86 87 87 ]
    Scores of Grade B performers are:
    [ 67 74 76 77 ]
    Scores of Grade C performers are:
    [ 54 54 55 65 ]
    Scores of Grade D performers are:
    [ 39 45 45 46 ]
    Scores of Grade E performers are:
    [ 24 24 30 33 ]
    Scores of Grade F performers are:
    [ 13 ]
    BUILD SUCCESSFUL (total time: 10 seconds)



    3. run:
    Please enter test scores that range from 0 to 100 in a row
          Separate each grade with a White space.
          INFO: Scores with decimals will be rounded to the closest whole number.

    87 87 67 54 87 23 22.7
    Total count of scores entered is: 7 , approx. number of scores in each range is: 1
    [ 23 23 54 67 87 87 87 ]
    Following is the break-up of the input scores:
    **********************************************
    Scores of Grade A performers are:
    [ 87 87 87 ]
    Scores of Grade B performers are:
    [ 67 ]
    Scores of Grade C performers are:
    [ 54 ]
    Scores of Grade D performers are:
    [ 23 23 ]
    Scores of Grade E performers are:
    [ ]
    Scores of Grade F performers are:
    [ ]
    BUILD SUCCESSFUL (total time: 19 seconds)







    4. run:
    Please enter test scores that range from 0 to 100 in a row
    Separate each grade with a White space.
    INFO: Scores with decimals will be rounded to the closest whole number.

    100 55 76
    Total count of scores entered is: 3 , approx. number of scores in each range is: 1
    [ 55 76 100 ]
    Following is the break-up of the input scores:
    **********************************************
    Scores of Grade A performers are:
    [ 100 ]
    Scores of Grade B performers are:
    [ 76 ]
    Scores of Grade C performers are:
    [ 55 ]
    Scores of Grade D performers are:
    [ ]
    Scores of Grade E performers are:
    [ ]
    Scores of Grade F performers are:
    [ ]
    BUILD SUCCESSFUL (total time: 13 seconds)







    5. run:
    Please enter test scores that range from 0 to 100 in a row
    Separate each grade with a White space.
    INFO: Scores with decimals will be rounded to the closest whole number.

    87 45.8 23.9 67 76 54 45 77.88 65.4 33 12.9 39 55.46798 29.888 73.6 45 54 23.9 87 86 76.5 70.5 69.5 69.4 99 92 91 91 89 85 83 82 80 79 78 78 77 76 75 74 62 55 43 20
    Total count of scores entered is: 44 , approx. number of scores in each range is: 8
    [ 13 20 24 24 30 33 39 43 45 45 46 54 54 55 55 62 65 67 69 70 71 74 74 75 76 76 77 77 78 78 78 79 80 82 83 85 86 87 87 89 91 91 92 99 ]
    Following is the break-up of the input scores:
    **********************************************
    Scores of Grade A performers are:
    [ 86 87 87 89 91 91 92 99 ]
    Scores of Grade B performers are:
    [ 78 78 78 79 80 82 83 85 ]
    Scores of Grade C performers are:
    [ 71 74 74 75 76 76 77 77 ]
    Scores of Grade D performers are:
    [ 54 55 55 62 65 67 69 70 54 ]
    Scores of Grade E performers are:
    [ 24 30 33 39 43 45 45 46 24 ]
    Scores of Grade F performers are:
    [ 13 20 ]
    BUILD SUCCESSFUL (total time: 4 seconds)
     
    Ravi Desigan
    Ranch Hand
    Posts: 59
    1
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator

    Tim Cooke wrote:They do have a point.

    Answer me this: To split up a list of items into separate buckets where each bucket contains 20% of the items in the original list, how many buckets would you require?

    Then answer me this: How many buckets (grades) does your code have?

    Have you spotted where you went wrong yet?



    Honestly not sure what is wrong with my code still...

    Is it ok if I submit my email here? (If someone cannot post an answer here, I welcome them to write to me in privacy.)

    I truly appreciate your input of time to clarify things to me. 
     
    Ravi Desigan
    Ranch Hand
    Posts: 59
    1
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator

    Campbell Ritchie wrote:

    Ravi Desigan wrote:. . . . The specification mentions Grades A through F.

    Not in what you quoted; there is no E.

    . . . If there are two (or more) scores that are identical, then those identical numerical scores must always receive the same grade, . . . .

    Where have you implemented that part?



    Ritchie,

    Within the method fetchNperformersFromMasterList, I have implimented the duplicate check logic.

    That is, given a sorted list of numbers, for example, 92, 91, 89, 87, the method would look for the ending number, in this case, 87 in the remaining master list... and it will loop through until dups are found. It will count the number of dups (87) in the masterlist, count these duplicates, and add that many 87's to the first list and subtract (remove) them from the masterlist.
     
    Ravi Desigan
    Ranch Hand
    Posts: 59
    1
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator

    Liutauras Vilda wrote:1. After you get all code working, the other problem might be that you haven't demonstrated any written tests for your code. Have you got them? If no, you can not state your code works and gives correct answer.

    2.

    Ravi Desigan wrote:fetchNperformersFromMasterList method does the core functionality


    instructions wrote:Write a function that takes an arbitrary (possibly unsorted) score list...


    You don't have any method which would take List as an argument. For some reason you assumed list must be an ArrayList. Which is not what is specified. Plus you added some extra parameter.

    Some of the things you need to take from this interview are:
  • Need to do exactly what is asked, no more - failed on that
  • Code must be tested, proof provided - failed on that
  • Code must be readable - failed on that
  • Code must be well formatted, formatting consistent - failed on that

  • This code is over-complicated. You were asked to write a function/method, one in total, which is accessible to the wide world, meaning one public function and the rest methods private which would serve as helper methods.

    3. Less than optimum outcome is also an outcome - so try to take as much as you can from it. I assume it was home interview exercise, meaning you had fair amount of time, so you had to come up with much more solid solution and code in general.



    Yes, I agree -- I can see that they wanted a method that takes the list alone as you specify whereas in my case, I have an arraylist and several other parameters too! I completely missed this point! Thanks for that.
     
    Liutauras Vilda
    Marshal
    Posts: 5804
    401
    BSD
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator

    Ravi Desigan wrote:Yes, I agree -- I can see that they wanted a method that takes the list alone as you specify whereas in my case, I have an arraylist and several other parameters too! I completely missed this point!


    List and ArrayList are different. List is an interface, while ArrayList is a an implementation of List interface. When you program, program to an interface (or abstractions). Imagine if user were to pass in a LinkedList. Even though if you have had an ArrayList a singular parameter - they couldn't use your method, because your implementation require specifically ArrayList, while requirement says List. You can check how many different List implementations are.

    But to be honest I don't think that was your main problem. Such things might would be forgiven taking in account that there is a time pressure and in general non natural conditions. In general I find your code hardly readable. What would be expected from you in most companies, that such requirements could be inferred  fairly easily reading solely your code without even reading any instructions (as these are fairly simple) - which is not the case.

    Remember, that in most cases code needs to be maintained and read by other programmers. If the code works, but is hardly understandable, or takes good amount of time to decipher what it does - that is a bad sign. As a proof of that is that Campbell couldn't spot instantly your implemented part about duplicated marks, while that supposed to be obvious by looking to the code.
     
    Ravi Desigan
    Ranch Hand
    Posts: 59
    1
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    I don't think there is a way to cut-short the logic from the way I have implemented... which is why I posted this question.

    If anyone can write it in a better way, they can definitely show it to me, I truly welcome it.

    Even if it is not the entire code, but just part of the code that could be bettered, I would be happy to hear it.
     
    Tim Cooke
    Marshal
    Posts: 4425
    283
    Clojure IntelliJ IDE Java
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Have another look at the requirements and tell me how many grade categories are specified?

    * A score in the top 20% of all scores is an A.
    * A score in the next 20% of scores is a B.
    * A score in the next 20% of scores is a C.
    * A score in the next 20% of scores is a D.
    * A score in the bottom 20% of scores is an F.


    I count 5. A, B, C, D, and F.

    How many does your code have?
     
    Ravi Desigan
    Ranch Hand
    Posts: 59
    1
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator

    Tim Cooke wrote:Have another look at the requirements and tell me how many grade categories are specified?

    * A score in the top 20% of all scores is an A.
    * A score in the next 20% of scores is a B.
    * A score in the next 20% of scores is a C.
    * A score in the next 20% of scores is a D.
    * A score in the bottom 20% of scores is an F.


    I count 5. A, B, C, D, and F.

    How many does your code have?



    OH...............! I see my mistake. I just assumed the examiner made a mistake in omitting grade E earlier. Feeling silly!
     
    Campbell Ritchie
    Marshal
    Posts: 59437
    187
    • Likes 1
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator

    Ravi Desigan wrote:. . . . I just assumed the examiner made a mistake in omitting grade E earlier. . . .

    But it said divide into 20% segments, so that makes five parts.

    If you get that sort of thing again, assume that what the paper says is correct. Unless there is something impossible to implement, do exactly what it says. If there is anything you find strange, tell them that when you hand in the code.
     
    Master Rancher
    Posts: 2712
    92
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    There is something else wrong, if I interprete the question correctly. It is clear when you look at your test outcome 3.
    You are given the list {87, 87, 87, 67, 54, 23, 23}. The first three  numbers form 42% of the scores, 67 comes in the category 57%. 67 is therefore in the range 40% - 60% and should get a grade C. It becomes even more clear in test 4.
     
    Greenhorn
    Posts: 19
    Eclipse IDE Java VI Editor
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    My question is related to the logistics of the request. Did they provide a laptop and request you to do it in the interview? What was the time frame for this request?
     
    Hey! Wanna see my flashlight? It looks like this tiny ad:
    Why should you try IntelliJ IDEA ?
    https://coderanch.com/wiki/696337/IntelliJ-IDEA
    • Post Reply Bookmark Topic Watch Topic
    • New Topic
    Boost this thread!