Skip to main content
added 70 characters in body
Source Link
asteri
  • 4.9k
  • 5
  • 24
  • 48

The algorithm itself looks solid, and your code looks good! I just wanted to provide some general style tips for Java.

First, why are you performing the core of your logic in the test1 constructor? That's highly irregular for Java. A constructor in general should only perform processing to initialize values unique to each instance of the class. It would be much more object-oriented to either move all that code directly into your main(String[] args) method or into a different static method. Having a test1 instance doesn't even make sense here.

Second (and much less importantly), you are breaking with several Java style conventions. I'm assuming you come from a Python background, because your code "feels" very Pythonic. That's not a bad thing, but they're two very different languages. Python attempts to always be short and to the point, while Java emphasizes verbosity and long, descriptive names for things. As such, here are a few tips to make your code more Java-esque (if you care):

  • Class names should have each word capitalized, like in ThisTestClassName. So yours should be Test1 and Stats.
  • Class names and variable names are also generally not abbreviated. So for example, your stats class would likely be called Statistics.
  • Likewise, your method names would likely be renamed to something more descriptive. While anyone with a statistics background will know what you mean by stddev, it's standard for Java methods to be unabbreviated verb phrases. So it would be more appropriate to rename this method as getStandardDeviation(). Believe it or not, this really does make a difference in code readability. For example, when scanning your code, I was very confused by this line at first (Math.sqrt(var(a))) because it seemed like you were trying to do some odd JavaScript casting of a at a glance. A more descriptive method name like getVariation() would have easily avoided that confusion.
  • To a lesser degree the "make things more descriptive" points from above apply to variable names as well, though it's much more common (if still frowned upon) to see this in local scope variables. For example, your code gets a little obfuscated in your test1() constructor because you use at least four variables which are only one letter: i, k, r, and s. In theory, with well-chosen variable names, I should be able to understand what the code is trying to do at any point without much surrounding context. That's the goal of making "self-documenting code".

And finally, one small tip which might clean up your code a little bit concerning this line:

for (Double s : dmap.values().toArray(new Double[dmap.size()]))

Here you can actually just use .toArray(new Double[0]) if you feel that would make your code less wordy. As you can see in the documentation, you don't actually have to pass in an array of the appropriate size to the toArray() method; it only uses your array for infer the type to return to you. So by just initializing an empty, zero-length array like this, you may be able to improve code readability a bit (and performance by just a small amount, by saving on memory allocation). There's no consensus or standard on this. Just personal preference.

The algorithm itself looks solid, and your code looks good! I just wanted to provide some general style tips for Java.

First, why are you performing the core of your logic in the test1 constructor? That's highly irregular for Java. A constructor in general should only perform processing to initialize values unique to each instance of the class. It would be much more object-oriented to either move all that code directly into your main(String[] args) method or into a different static method. Having a test1 instance doesn't even make sense here.

Second (and much less importantly), you are breaking with several Java style conventions. I'm assuming you come from a Python background, because your code "feels" very Pythonic. That's not a bad thing, but they're two very different languages. Python attempts to always be short and to the point, while Java emphasizes verbosity and long, descriptive names for things. As such, here are a few tips to make your code more Java-esque (if you care):

  • Class names should have each word capitalized, like in ThisTestClassName. So yours should be Test1 and Stats.
  • Class names and variable names are also generally not abbreviated. So for example, your stats class would likely be called Statistics.
  • Likewise, your method names would likely be renamed to something more descriptive. While anyone with a statistics background will know what you mean by stddev, it's standard for Java methods to be unabbreviated verb phrases. So it would be more appropriate to rename this method as getStandardDeviation(). Believe it or not, this really does make a difference in code readability. For example, when scanning your code, I was very confused by this line at first (Math.sqrt(var(a))) because it seemed like you were trying to do some odd JavaScript casting of a at a glance. A more descriptive method name like getVariation() would have easily avoided that confusion.
  • To a lesser degree the "make things more descriptive" points from above apply to variable names as well, though it's much more common (if still frowned upon) to see this in local scope variables. For example, your code gets a little obfuscated in your test1() constructor because you use at least four variables which are only one letter: i, k, r, and s. In theory, with well-chosen variable names, I should be able to understand what the code is trying to do at any point without much surrounding context. That's the goal of making "self-documenting code".

And finally, one small tip which might clean up your code a little bit concerning this line:

for (Double s : dmap.values().toArray(new Double[dmap.size()]))

Here you can actually just use .toArray(new Double[0]) if you feel that would make your code less wordy. As you can see in the documentation, you don't actually have to pass in an array of the appropriate size to the toArray() method; it only uses your array for infer the type to return to you. So by just initializing an empty, zero-length array like this, you may be able to improve code readability a bit (and performance by just a small amount, by saving on memory allocation).

The algorithm itself looks solid, and your code looks good! I just wanted to provide some general style tips for Java.

First, why are you performing the core of your logic in the test1 constructor? That's highly irregular for Java. A constructor in general should only perform processing to initialize values unique to each instance of the class. It would be much more object-oriented to either move all that code directly into your main(String[] args) method or into a different static method. Having a test1 instance doesn't even make sense here.

Second (and much less importantly), you are breaking with several Java style conventions. I'm assuming you come from a Python background, because your code "feels" very Pythonic. That's not a bad thing, but they're two very different languages. Python attempts to always be short and to the point, while Java emphasizes verbosity and long, descriptive names for things. As such, here are a few tips to make your code more Java-esque (if you care):

  • Class names should have each word capitalized, like in ThisTestClassName. So yours should be Test1 and Stats.
  • Class names and variable names are also generally not abbreviated. So for example, your stats class would likely be called Statistics.
  • Likewise, your method names would likely be renamed to something more descriptive. While anyone with a statistics background will know what you mean by stddev, it's standard for Java methods to be unabbreviated verb phrases. So it would be more appropriate to rename this method as getStandardDeviation(). Believe it or not, this really does make a difference in code readability. For example, when scanning your code, I was very confused by this line at first (Math.sqrt(var(a))) because it seemed like you were trying to do some odd JavaScript casting of a at a glance. A more descriptive method name like getVariation() would have easily avoided that confusion.
  • To a lesser degree the "make things more descriptive" points from above apply to variable names as well, though it's much more common (if still frowned upon) to see this in local scope variables. For example, your code gets a little obfuscated in your test1() constructor because you use at least four variables which are only one letter: i, k, r, and s. In theory, with well-chosen variable names, I should be able to understand what the code is trying to do at any point without much surrounding context. That's the goal of making "self-documenting code".

And finally, one small tip which might clean up your code a little bit concerning this line:

for (Double s : dmap.values().toArray(new Double[dmap.size()]))

Here you can actually just use .toArray(new Double[0]) if you feel that would make your code less wordy. As you can see in the documentation, you don't actually have to pass in an array of the appropriate size to the toArray() method; it only uses your array for infer the type to return to you. So by just initializing an empty, zero-length array like this, you may be able to improve code readability a bit (and performance by just a small amount, by saving on memory allocation). There's no consensus or standard on this. Just personal preference.

Source Link
asteri
  • 4.9k
  • 5
  • 24
  • 48

The algorithm itself looks solid, and your code looks good! I just wanted to provide some general style tips for Java.

First, why are you performing the core of your logic in the test1 constructor? That's highly irregular for Java. A constructor in general should only perform processing to initialize values unique to each instance of the class. It would be much more object-oriented to either move all that code directly into your main(String[] args) method or into a different static method. Having a test1 instance doesn't even make sense here.

Second (and much less importantly), you are breaking with several Java style conventions. I'm assuming you come from a Python background, because your code "feels" very Pythonic. That's not a bad thing, but they're two very different languages. Python attempts to always be short and to the point, while Java emphasizes verbosity and long, descriptive names for things. As such, here are a few tips to make your code more Java-esque (if you care):

  • Class names should have each word capitalized, like in ThisTestClassName. So yours should be Test1 and Stats.
  • Class names and variable names are also generally not abbreviated. So for example, your stats class would likely be called Statistics.
  • Likewise, your method names would likely be renamed to something more descriptive. While anyone with a statistics background will know what you mean by stddev, it's standard for Java methods to be unabbreviated verb phrases. So it would be more appropriate to rename this method as getStandardDeviation(). Believe it or not, this really does make a difference in code readability. For example, when scanning your code, I was very confused by this line at first (Math.sqrt(var(a))) because it seemed like you were trying to do some odd JavaScript casting of a at a glance. A more descriptive method name like getVariation() would have easily avoided that confusion.
  • To a lesser degree the "make things more descriptive" points from above apply to variable names as well, though it's much more common (if still frowned upon) to see this in local scope variables. For example, your code gets a little obfuscated in your test1() constructor because you use at least four variables which are only one letter: i, k, r, and s. In theory, with well-chosen variable names, I should be able to understand what the code is trying to do at any point without much surrounding context. That's the goal of making "self-documenting code".

And finally, one small tip which might clean up your code a little bit concerning this line:

for (Double s : dmap.values().toArray(new Double[dmap.size()]))

Here you can actually just use .toArray(new Double[0]) if you feel that would make your code less wordy. As you can see in the documentation, you don't actually have to pass in an array of the appropriate size to the toArray() method; it only uses your array for infer the type to return to you. So by just initializing an empty, zero-length array like this, you may be able to improve code readability a bit (and performance by just a small amount, by saving on memory allocation).