• Post Reply Bookmark Topic Watch Topic
  • New Topic

Help with logic, optimizing code and prime numbers  RSS feed

 
Charles D. Ward
Ranch Hand
Posts: 99
1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi everyone,

First of all, I'm a newbie and it's going to be very obvious if you read my code. I'm also really bad at math. Now, on to my question(s):

I made a class named Primos (spanish for Prime as in prime numbers). I wanted to do this exercise by myself, so I didn't look for an already existing class or method that deals with this. I wanted to make my own class from scratch. It has three methods:

a) public static boolean esPrimo (int number) //esPrimo is spanish for 'isPrime'
Returns a boolean indicating if the number supplied is a prime number or not.

b) public static int[] entre (int inicio, int fin) //entre is spanish for 'Between'. Inicio = begin, Fin = end
Returns an int array containing all the prime numbers between the supplied numbers Inicio and Fin

c) public static int[] hasta (int fin) //hasta is spanish for 'up to'
Returns an int array containing all the prime numbers from 0 (or 2 if you will, which is the first prime number) up to the number specified


I've run a fair amount of tests with my class and as far as I can tell it is working correctly, but there are a few things I'd like cleared up.

I made the methods static because I wanted to use them without having to create an object of the Primos class, so I can just call them like this for example:



Question 1) Is my understanding of the use of static methods correct? Are there any disadvantages in making these methods static?


Now this is the code of my Primos class. I can't help but think this is horrible code for some reason, even though it works as desired as far as I can tell, so I'd really appreciate some tips on how can I make this code more concise and if there are some obvious logic flaws you can help me improve.



In the last 2 methods I use an ArrayList<Integer> but I end up converting it into an int array which is what the method returns. Why? Because I couldn't find a way to dimension the int array without knowing beforehand how many ints it should hold, so I used an ArrayList. I chose to return an int array instead of an ArrayList<Integer> because I didn't want to depend on importing the ArrayList class in other projects where I'd use my Primos class only so I could use the entre and hasta methods, but this reeks of bad design imo, and since my Java knowledge is extremely basic, I'm not seeing how can I make this more efficient and less well.. dumb.

Thank you for your time beforehand and sorry for my horrible english (and my horrible code and math skills ).
 
manish ghildiyal
Ranch Hand
Posts: 136
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
You can call toArray() method on arraylist.

Manish
 
Winston Gutkowski
Bartender
Posts: 10575
66
Eclipse IDE Hibernate Ubuntu
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Charles D. Ward wrote:First of all, I'm a newbie and it's going to be very obvious if you read my code. I'm also really bad at math.

But you are NOT bad at explaining your problem, which means that eventually you'll probably be a good programmer.
Very nice explanation. Well done.

Question 1) Is my understanding of the use of static methods correct? Are there any disadvantages in making these methods static?

Yes, several; but the main one is that depending on how you write them, it may be more difficult to make your methods Thread-safe.
If you allow each client to make their own Primos instance, then they can execute it independently of any other.

Now this is the code of my Primos class. I can't help but think this is horrible code for some reason, even though it works as desired as far as I can tell, so I'd really appreciate some tips on how can I make this code more concise and if there are some obvious logic flaws you can help me improve.

OK:

1. if (numero == 2)
There's nothing wrong with this, but why not make it a "numero divides by 2" check? That way, you eliminate 50% of possible values immediately.
And just FYI, the fastest way to check if a number divides specifically by 2 is "(numero & 1) == 0".
I'll leave you to find out why.

2. for (int i = 3; i < numero; i = i + 2) {
This loop is doing far too much. You're checking factors of numero, remember? So, when do you think you can stop?

3. public static int[] hasta(int fin)
Isn't this almost a repeat of your entre() code? Have a think how you could get the two methods to work together.

There are a few other things, but that should be enough for the moment.

HIH

Winston
 
fred rosenberger
lowercase baba
Bartender
Posts: 12565
49
Chrome Java Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
82 lines of code an no comments - bad style.

I always comment my code - even code I'm just writing to screw around with.
 
Charles D. Ward
Ranch Hand
Posts: 99
1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Manish, I completely forget about the toArray() method. Thank you!

Winston, that is why it's so helpful to have a fresh pair of eyes to look for improvements in one's code. Thank you very much for all the tips. I'll be looking into them all today.

Fred, I usually comment my code, a lot, but this time I wanted to remove 'noise' before posting here. I'll leave only the necessary comments in the code next time. Cheers.

Thank you all for your help. I'll post back later with what I could come up with following your advice.
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!