• Post Reply Bookmark Topic Watch Topic
  • New Topic

Tips for improving my code  RSS feed

 
Jon Davis
Greenhorn
Posts: 14
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I am trying to help my brother with his homework for Java. This is what the assignment says

In this assignment, you will develop an array utility class and use the class in your program. The array utility class is called ArrayUtil; it contains the following methods:
•arrayRnd: takes an integer array as its input parameter, fills the array with randomly generated integers, which are in the range of 1 to 1000.
•arrayEven: takes an integer array as its input parameter, fills the array with randomly generated even integers, which are in the range of 1 to 1000.
•arrayOdd: takes an integer array as its input parameter, fills the array with randomly generated odd integers, which are in the range of 1 to 1000.
•arrayMax: takes an integer array as its input parameter, and returns the maximum number in the array.
•arrayMin: takes an integer array as its input parameter, and returns the minimum number in the array.
•arrayAvg: takes an integer array as its input parameter, and returns the average of the numbers in the array.
•arrayDiv: takes an integer array as its input parameter, and fills the array with randomly generated integers that are divisible by 3. These random numbers are in the range of 1 to 1000.
•arrayDisplay: takes an integer array as its input parameter, and displays its components.

And this is what he and I have came up with. Does anyone have any suggestions to improve it?


It works properly and I get the desired results, but I was wondering if there was anything I could do differently that would improve it.
 
Piet Souris
Master Rancher
Posts: 2044
75
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
hi Jon,

I have indeed some suggestions.

First of all, at a first quick read I think the methods in the ArrayUtil class are fine, So, half of the
job done.

Then: your ArrayUtil does not meet the rewuirements. These cleaarly indicate that ArraUtil must take an integer array as input.
What you are doing here is filling and returning a member of your ArrayUtil-object.

The diea is that you do not start from a member-array, but that the array-to-fill is delivered to ArrayUtil, as said, as input parameter.

So you would get something like: (in your ArrayUtil-clas)



or alternatively



And finally some questions: do you think you need many ArrayUtil-objects? If not, then
could you make the required methods static? For an idea: look at the Math-class API

Greetings,
Piet
 
Dennis Grimbergen
Ranch Hand
Posts: 159
IntelliJ IDE Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
These methods are a good candidate for being static. A lot of utility classes only have static methods, like java.util.Arrays. Each method invocation is not depending on something else (in that class). Don't forget to make a private constructor, so you can't (accidently) make an instance of it.
And like Piet said, you should use those int[] arrays as a method parameter, as required by your specification. I would even use the exact method names as specified. So use arrayRnd instead of arrayRandom etc.
 
Jon Davis
Greenhorn
Posts: 14
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
First let me say thank you both for your feedback, I am trying to teach myself to keep up with my brother who is attending the class, but I am not doing so good, and your feedback is a great help. I think this is a little bit better, and btw thank you for pointing out that I was missing the input parameter as an integer array , Idk how I missed that. Also I made everything static, I am a little confused about making a private constructor though, I think this is what I was supposed to do, but I am a little confused by what that is actually doing.
 
Dennis Grimbergen
Ranch Hand
Posts: 159
IntelliJ IDE Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
You should add the private constructor to the ArrayUtil class.
And then for your ArrayTest main method:
 
Jon Davis
Greenhorn
Posts: 14
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Dennis Grimbergen wrote:You should add the private constructor to the ArrayUtil class.
And then for your ArrayTest main method:

I think I have it now, I understand what you were saying, sometimes I am a little slow.

And then my main method should look like this.
 
Stuart A. Burkett
Ranch Hand
Posts: 679
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Jon Davis wrote:thank you for pointing out that I was missing the input parameter as an integer array

The requirements also say that you should be filling that array with the generated values. You are creating a new array and returning it - that is not the same thing.

Note also that theoretically your arrayEven, arrayOdd and arrayDiv methods may never return or could take a long time to run. e.g. if all the random numbers generated are even, then arrayOdd would never complete. You probably don't need to worry about this, but finding a way to avoid this is a potential enhancement in future versions.
 
fred rosenberger
lowercase baba
Bartender
Posts: 12565
49
Chrome Java Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
my PERSONAL opinion...

a lot of the comments are unnecessary - and some are actually wrong. for example:

This method does not CREATE an array at all. It POPULATES one, but the array was created elsewhere. Further, this method in no way enforces the "20 places". The array can be of ANY size.

Another personal opinion....I would never do this:

i ALWAYS do this:

(use whatever brace style you prefer).
You should always use braces, even though they aren't needed. if you don't someday you will be burned when you have to add something else. I guarante you will do this at least once:

and get confused why it doesn't print what you expect.
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!