• 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:
  • Campbell Ritchie
  • Jeanne Boyarsky
  • Ron McLeod
  • Paul Clapham
  • Liutauras Vilda
Sheriffs:
  • paul wheaton
  • Rob Spoor
  • Devaka Cooray
Saloon Keepers:
  • Stephan van Hulst
  • Tim Holloway
  • Carey Brown
  • Frits Walraven
  • Tim Moores
Bartenders:
  • Mikalai Zaikin

code critique

 
Greenhorn
Posts: 6
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hi, I would appreciate it a great deal, if someone would please, constructively, critique my code ?  

Required methods :
public static double min(double[][] a)
public static double max(double[][] a)
public static double mean(double[][] a)
public static double median(double[][] a)

Which compute the minimum, the maximum, the mean value (the average value), and the median reps.
NB. For the median of an array with an even number of numbers in total return the average of the two medianvalues.



 
author & internet detective
Posts: 41860
908
Eclipse IDE VI Editor Java
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Overall, your code is good. A few minor things you can improve

  • In min() you have an if statement that throws an exception. That's fine. Then immediately after you have another if. But you already know count(a) >=1 because the code didn't throw an exception
  • The indentation is messed up in the beginning of a few methods, but that might be from posting
  • One letter variable names are good for loop variables. They aren't common for method parameters. Consider changing "a" to "array"
  • In median(), you have a loop that counts the number of elements. But there's already a method to do that so you could call it form there.

  •  
    Marshal
    Posts: 8857
    637
    Mac OS X VI Editor BSD Java
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator

    K Lana wrote:


    K Lana, do you really need to iterate over each element and increment count that way?

    Do you think it could work having something similar to:


    I think your way would make much more sense if somehow you'd check whether elements aren't default, but that would be more apparent if an array would contain not the primitives.
     
    Liutauras Vilda
    Marshal
    Posts: 8857
    637
    Mac OS X VI Editor BSD Java
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator

    K Lana wrote:


    Consider renaming method "count" which is responsible for counting how many elements an array of array has. The main reason for that is..., look at the method "mean" you got here. It looks so confusing that you have a method call "count" as well as variable name "count". Both of them are from completely different contexts.

    Method "count" I'd personally name something along the line: numberOfElementsIn(double[][])

    And so you'd have:
     
    Bartender
    Posts: 2911
    150
    Google Web Toolkit Eclipse IDE Java
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator
    One way of implementing would be to use Streams in Java 8. Here's one suggestion from my end that matches up to your method behavior :
    I think this can be made shorter...
     
    Bartender
    Posts: 5465
    212
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator
    Far out of scope, but I tried to generalize Salvins method. Unfortunately, that turned out a little more complicated than I thought. But it is always good to have some practice, so here goes:

    Typically a case of: surgery: succes, patient: dead.
     
    Master Rancher
    Posts: 4806
    72
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator
    Yeah, the convenience methods on DoubleStream are too useful to pass up here.  My version:


    I also had a slightly longer but theoretically more efficient version of mean:
     
    salvin francis
    Bartender
    Posts: 2911
    150
    Google Web Toolkit Eclipse IDE Java
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator

    Piet Souris wrote:... Typically a case of: surgery: succes, patient: dead.

    I said shorter !!

    I like Mike's approach using flatMapToDouble.
     
    Piet Souris
    Bartender
    Posts: 5465
    212
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator
    That I didn't think of flatmapToDouble.... or did I?

    My intention was to come up with only one method that did it all. Using different methods for all the different results is too easy to have much fun.
     
    Liutauras Vilda
    Marshal
    Posts: 8857
    637
    Mac OS X VI Editor BSD Java
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator
    Knowing academia to some extent, I think OP needs a bit more habitual approach to the problem, similar to that in original post. Mike's approach is indeed neat, but perhaps too cool to be of use
     
    Piet Souris
    Bartender
    Posts: 5465
    212
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator
    Yep,

    OP asked for critique, which he got, while the last replies were about doing it in a different way. That's why I wrote: Far out of scope, but indeed that is not a strong excuse.
     
    salvin francis
    Bartender
    Posts: 2911
    150
    Google Web Toolkit Eclipse IDE Java
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator

    Piet Souris wrote:That I didn't think of flatmapToDouble.... or did I?..


    oh !! I totally missed that  
    This quarantine is now getting to my head apparently  
     
    Marshal
    Posts: 79179
    377
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator
    Why are you declaring an unchecked exception with throws?
     
    salvin francis
    Bartender
    Posts: 2911
    150
    Google Web Toolkit Eclipse IDE Java
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator

    Campbell Ritchie wrote:Why are you declaring an unchecked exception with throws?

    Because I blindly copied OP's method declaration.  
    Campbell is right, the throws is not needed.
     
    Campbell Ritchie
    Marshal
    Posts: 79179
    377
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator

    salvin francis wrote:. . . the throws is not needed.

    ...but the documentation comments should include the @throws tag (or @exception, which does the same).
     
    K Lana
    Greenhorn
    Posts: 6
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator
    Thank you all for taking the time !  

  • Jeanne Boyarsky - thanks for the recommendations & the book! it's been a game changer for me.
  • Liutauras Vilda - thank you for changing the way I was thinking about all of this. I hadn't even questioned why I was iterating through. I have made the changes.  
  • Salvin francis, Piet Souris , Mike Simmons - Thanks for the suggestions, I am working my way through the OCP Java 8 book, once I am familiar with streams I will try it out.  



  •  
    Sheriff
    Posts: 17644
    300
    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
    Your code formatting is still all over the place. If you're using an IDE, there should be a way to autoformat your code.

    How to autoformat code in Eclipse
    How to autoformat code in IntelliJ IDEA
    How to autoformat code in NetBeans
    How to autoformat Java code online

    Formatting is not just a matter of style and aesthetics. It shows attention to detail. It shows that you care that your code is readable to others. It also helps prevent bugs and makes it easier to find bugs.
     
    Don't get me started about those stupid light bulbs.
    reply
      Bookmark Topic Watch Topic
    • New Topic