Win a copy of Rust Web Development this week in the Other Languages forum!
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Tim Cooke
  • Campbell Ritchie
  • Ron McLeod
  • Liutauras Vilda
  • Jeanne Boyarsky
Sheriffs:
  • Junilu Lacar
  • Rob Spoor
  • Paul Clapham
Saloon Keepers:
  • Tim Holloway
  • Tim Moores
  • Jesse Silverman
  • Stephan van Hulst
  • Carey Brown
Bartenders:
  • Al Hobbs
  • Piet Souris
  • Frits Walraven

Review my solution?

 
Ranch Hand
Posts: 327
6
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Whoops just realized this is posted in the wrong forum.  I do not see an option to delete it so I'm going to look for a way to ask for it to be moved.



I finished an exercise on codewars.com  ( https://www.codewars.com/kata/550498447451fbbd7600041c/train/java  ) and there are a lot of disagreements / discussion it seems around other people's solution.  I'm just wondering if you would please critique my solution?    One thing which was not agreed on is the Arrays.sort().   Supposedly it's not cheap ( what does that mean?) .  Is it really not a good option?

 
lowercase baba
Posts: 13018
66
Chrome Java Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
When talking about something being not cheap in a program, it means the cpu must do a lot of work, take a lot of time, and/or use a lot of memory.  Now, "a lot" is a relative term.  small arrays will take milliseconds.  

even "good" is a loaded term.  Generally, if you don't have specific, well defined specs on time, then who cares?  If it's for a user interface, they won't notice the difference between .1 seconds and .2 seconds to do something like this.  If it's a missile guidance system, then a tenth of a second can make a huge difference.  barring that kind of requirement, simple, clean code is ALWAYS the right answer.  I'll take slower code I can read an understand over faster code that takes a while to figure out some trick the coder used every day of the week.


 
Sheriff
Posts: 7113
184
Eclipse IDE Postgres Database VI Editor Chrome Java Ubuntu
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
The implementation notes on Arrays.sort() says that its execution time is O(n log(n)) a lot of the time ("on many data sets.")  That's a good performance for a sort, so I wouldn't say that sort() is "not cheap."  How much memory it uses I don't know, but memory is literally cheap these days, so it's not as much of a concern.
 
Marshal
Posts: 74632
335
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Isn't it a quicksort for an int[]? In which case wouldn't the memory requirement be the same as that for the original array?
I think trying to find whether two arrays are anagrams of each other can readily be done by sorting. Probably a simpler and therefore less error‑prone solution than some others. What solutions did other people propose themselves?
 
Lisa Austin
Ranch Hand
Posts: 327
6
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Campbell Ritchie wrote:Isn't it a quicksort for an int[]? In which case wouldn't the memory requirement be the same as that for the original array?
I think trying to find whether two arrays are anagrams of each other can readily be done by sorting. Probably a simpler and therefore less error‑prone solution than some others. What solutions did other people propose themselves?



My solution ( but slightly different ) was voted as best practice but the discussion about how sorting isn't cheap was under it.  Then it was discussed under the second most "Best Practice" and a suggestion to use a HashMap or a non-comparative sort such as Radix Sort .    

What do you think of my solution vs the first place "Best Practice" ?  It's similar I know so maybe not much can be said.  I was thinking mine maybe better because I'm checking for more than just null arrays ?




This was voted second as  "Best Practices" but a lot of disagreements as well.

 
Campbell Ritchie
Marshal
Posts: 74632
335
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Lisa Austin wrote:. . . voted as best practice

Well done

. . . checking for more than just null arrays ?

It is conventional for two nulls to be regarded as equal to each other. If you don't agree, I challenge you to take two nulls and find a difference between them And all that lot won't work because I didn't realise you were supposed to square the elements in array1. Sorry. Yes your line with n -> n * n will create a new array with n² throughout. Apart from the fact that I would format it like this...I don't see how I could improve on it. The only enhancement I would suggest is that you can test for the two arrays now being equal because you happen to get them in the right order already.

You mentioned use of a Map. You can enhance the counting exercise from the Java™ Tutorials. Count the elements in array2 and count the squares of the elements in array1 and simply compare the two maps with equals().
 
Sheriff
Posts: 16767
281
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Line 13 accesses the parameter b directly and since Arrays.sort() does so in place, the array in the calling code will experience the side effect of being sorted after the call is made. That is, if the array b is unsorted before the call to comp(), it will be sorted after. That's kind of a no-no since it's a side-effect that you wouldn't normally expect to happen nor necessarily want to happen.
 
Junilu Lacar
Sheriff
Posts: 16767
281
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Lisa Austin wrote: I was thinking mine maybe better because I'm checking for more than just null arrays?


Your code actually gives incorrect results per this requirement: "If a or b are nil (or null or None), the problem doesn't make sense so return false."

Your code will return true if a and b are null.
 
Campbell Ritchie
Marshal
Posts: 74632
335
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Junilu Lacar wrote:. . . If a or b are nil (or null or None) . . . return false . . ..

I didn't read the question as caarefully, so I didn't notie that. That might mean that empty arrays should be rejected as well as nulls.
 
Lisa Austin
Ranch Hand
Posts: 327
6
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Junilu Lacar wrote:

Lisa Austin wrote: I was thinking mine maybe better because I'm checking for more than just null arrays?


Your code actually gives incorrect results per this requirement: "If a or b are nil (or null or None), the problem doesn't make sense so return false."

Your code will return true if a and b are null.



Interesting.  I didn't catch that requirement AND I passed all the tests .  I probably should report it to CodeWars.  Thank you for pointing it out to me.
 
Campbell Ritchie
Marshal
Posts: 74632
335
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Lisa Austin wrote:. . . I didn't catch that requirement AND I passed all the tests . . . .

Hahahahahahahahaha! Well done! As you suspeted, it means their testing isn't complete.
 
Lisa Austin
Ranch Hand
Posts: 327
6
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Junilu Lacar wrote:Line 13 accesses the parameter b directly and since Arrays.sort() does so in place, the array in the calling code will experience the side effect of being sorted after the call is made. That is, if the array b is unsorted before the call to comp(), it will be sorted after. That's kind of a no-no since it's a side-effect that you wouldn't normally expect to happen nor necessarily want to happen.



So to not access the parameter b directory then I need to create a new int[] array and assign it the value b before sorting?
 
Junilu Lacar
Sheriff
Posts: 16767
281
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Yes, just like the other solution you showed does, the one you said got second "best practice".
 
You showed up just in time for the waffles! And this tiny ad:
Building a Better World in your Backyard by Paul Wheaton and Shawn Klassen-Koop
https://coderanch.com/wiki/718759/books/Building-World-Backyard-Paul-Wheaton
reply
    Bookmark Topic Watch Topic
  • New Topic