Need to get 100 numbers in my main method, send those numbers to my other method to split them into odd or even then say how many were even and how many were odd, right now it puts the number into 1's or 0's but I think the even and odd variables keep getting over written as when i run it, it gives 1 between the two once it reads the 100 numbers.

so help plx

where are you assigning oddCount a value? i see it defined, but i never see you assigning it any value, so it always stays 0.





Looks like you're assigning evenCount a new value with each iteration of the loop, either 0 or 1. how do you think you can add 0 or 1 to its existing value?

This post was edited by carteblanche on Nov 24 2014 08:31pm

There are a few variables in there right now that were just from prior attempts, also got it now, thanks for the tip in the right direction.

Heres a bit cleaner version of your code



The less amount of variables you need to use the cleaner your code will be.

I'm not familiar with Random class, all I use it for is Math.random, but it looks like your for loop just writes over your even number counter.

that is, isEven() returns 1 or 0, and then thats what you write... Next loop hapens, and whatever the last result was, is replaced with the new result...

You need to make your counters instance variables, IE declare them at the top of your class, not within a method. With the way you wrote your code the isEven method will ALWAYS return 0 or 1 because everytime you call that method the even/odd variables get reinitialized to 0. I would change your code by declaring evenCount/oddCount at the top of your class above your main method, also get rid of num1, even, odd these are unecessary. Also, remove the return from your isEven method, it is unnecessary. Instead of returning a value just increment the evenCount/oddCount variables which you'll be able to do if you declare them outside of your methods.

With the exception of carteblanche, everyone's advice in this thread is terrible. Global instance variables are always the goto for newbies. It is neither clean, nor good practice. It doesn't follow good OOP. If you don't understand these things, please don't try to "help" others, because you will only be reinforces terrible practices.

@OP, your initial approach is good. Your assignment is to count the number of even integers generated randomly out of a set of 100. You recognized an opportunity to encapsulate the logic for determining if a number is even in its own method. However, your method needs some work. You call it isEven, yet your method is not returning a value which represents if the paramater isEven. Typically, when a method has the prefix is, it is evaluating if some condition is true or false and returns a boolean. Therefore, your method would be better written to return true if the number isEven, and false if it is not (odd).



This will allow you to utilize this information in your for loop from your main method to calculate the number of odd and even numbers

I don't know if that was directed at me, but I just noticed what carteblanche pointed out and tried to explain it better. I guess I just didn't see where he was trying to count/ how. Also, why encapsulate an isEven method? just use if %2 == 0 wherever you need it, its not that long.

Wouldn't encapsulation be better demonstrated by making the variables private and making a few getter methods? Not that it's necessarily needed.

Couple of reasons. Why do you think he's using nextInt(10)? why does the Random class even come with it? just use (int)(nextDouble() * 10) wherever you need it, it's not that long. The answer is clarity. it's important to be able to understand what you're doing without needing to know every little details.

Lets say you have an app that has several hundred classes and i ask you to find me everywhere you check if a number is even. if you used a single method isEven everywhere, the answer is as simple as finding the references to it. I'm sure you're thinking "Why would I ever want to find the reference count?" Lets say we want to change the functionality. instead of checking if it's divisible by two, we want to check if it's divisible by three. How fun does that sound to change it over 300 classes? inside a method, you can quickly change it and rename it.

Another reason is type safety. java supports the modulus operator for doubles. 3.2 % 2 is not 0, therefore it's an odd number, right? Wrong, since "odd" only applies to integers. by creating a method that only accepts integers, you create a layer of safety.

The first reason was the major reason, but the others are valid reasons as well.



you are confusing static and non-static. the point of having member variables (variables outside of methods) is to maintain state. in OP's assignment, the number of even and odd only matter inside the main method. keep in mind that main is just a method. what do you think will happen if i use another class as my driver, then i call OddEven.main(..) twice? it will work great the first time, but your results will be wrong the second time because you're maintaining the state from the first time.

does that make sense?

To expand just a tiny bit more, as carteblanche did an amazing job, there is something in OOP known as the http://en.wikipedia.org/wiki/Single_responsibility_principle. In a nutshell, the description is in the title: every method, variable, class, etc should serve one single purpose. In context of OP's assignment, the main method should not have to know how to generate a Random number, determine if it is even/odd, and count each odd and even. That is 3 responsibilities. Actually, it is 4. Because the purpose of the main method is to provide an entry point into the application. It is a special method. That is it's single responsibility. It's job is to handle command-line arguments and forward them on to the rest of the application as needed. Everything else should be encapsulated within it's own method/class/variable which will serve it's own single purpose. Even though the isEven method is 1 mathematically operation, it provides greater clarity to anyone else reading the code. Everyone can see isEven and know that this is a method with a specific purpose, and can be reused whenever someone wants that functionality elsewhere. After all, that is the point of encapsulating methods: reducing redundant code.