Need help with java program.

[simplemind]

Ok so its apparent that my counter variables aren't incrementing. I'm not sure why. So, if anyone can point out what I'm missing I'd be very greatful.

  problem resolved. Deleting snippet to prevent accusations of cheating.

p.s. I don't think I need to paste the rest of the code for help with this particular problem. The rest of it is very long and tedious and this is only a very small part of what my program is supposed to achieve overall. Also, this is my first semester taking any sort of programming class, and the instructions say to use what we learned in ch.1-7 to solve the problem. So try to offer a solution kind of in the parameters of whats in this small snippet of code. Thanks!

[Mavecryst]

Apologies if I'm mistaken as I haven't programmed in awhile (just decided to take a look for fun to see if I could still "read" java) but umm..

your for loop seems wrong. shouldn't it be str++ at the end? you already have a limiter which is s.length()..

Also, this line double sNAverage = ((counterB/(counterA + counterB))*100); What happens if both counterA and CounterB are zero? Could that possibly happen during the course of execution? If so you'd have an error on the return of that calculation value, so that may be it too?

Hope someone more knowledgeable comes along if those two ideas don't help :P

[rkma]

It might be easier to help if it was clear what you were trying to do. What are you trying to compute?

[someotherguy]

stop in the debugger, and look at the variables.
Or add a simple WriteLine to print out the values and look at them.

debugging by yourself is going to teach you a lot more than someone giving you the answer.

but just because I don't like people who don't debug, I'm going to tell you what I think the issue is, so you don't learn debugging skills:
When you divide int by int, you get int.
So: double sNAverage = ((counterB/(counterA + counterB))*100);
You're dividing counterB by counterA+counterB, so lets say counterB is 10, and counterA+counterB is 20, the answer will be 0, because 0.5 is not valid value for int.
So you gotta do something like:
double sNAverage = ((((double)counterB)/(counterA + counterB))*100);

or:
double sNAverage = counterB * 100.0 /(counterA + counterB);

[Fortuitous]

Um, what are you trying to calculate? It looks like you are trying to determine the occurrence of the letter A and B in a string.

Your for loops are confusing me, it is comparing the 1st, 8th, 15th, 22st etc char to see if it is either A or B for the first loop.
Second loop is comparing 2nd, 9th, 16th, 23rd etc. I don't know if this is what you are trying to do or not.

I'm not sure what your assignment is asking, but to compare all elements in the string you need to fix the for loop increment.

for(int str = 0; str < s.length(); str++)
{
    a = Character.toUpperCase(s.charAt(str));
    if(a == 'A')
    counterA++;
    else if(a == 'B')
    counterB++;
    else
    counter++;
}

Don't forget, a string is an array of characters. Array's are indexed 0 through Size-1. So your for loop should start at 0 to check the first char.

Also as previously mentioned you need to type cast your integer values to double when dividing integers.

And,

int sNPercent = (int)(sNAverage + 0.5);

Will always take the floor; 9.5 will be 9 and so on.

[stuck4cash]

I see a few things...

firstly, I'd strongly recommend you don't use the variables "a" and "b" since they contain the next letter in string (well, the 1st, 8th or 2nd, 9th letter etc, as fortuitous explained).

secondly, we need some detail on what the purpose is. Unfortunately without knowing what you're trying to do, we can't help you out with logical errors.

I know that .toUpperCase will do what you want in this case, but is it possible that doing the following might be better?

for(int str = 0; str < s.length(); str++)
{
    if(a == 'A' || a == 'a')
    counterA++;
    else if(a == 'B' || b == 'b')
    counterB++;
    else
    counter++;
}

One last thing, loops are expensive, don't do them twice when you can do them once. Depending on what you're trying to achieve, this could be done multiple ways.

[simplemind]

I see a few things...

firstly, I'd strongly recommend you don't use the variables "a" and "b" since they contain the next letter in string (well, the 1st, 8th or 2nd, 9th letter etc, as fortuitous explained).

secondly, we need some detail on what the purpose is. Unfortunately without knowing what you're trying to do, we can't help you out with logical errors.

I know that .toUpperCase will do what you want in this case, but is it possible that doing the following might be better?

for(int str = 0; str < s.length(); str++)
{
    if(a == 'A' || a == 'a')
    counterA++;
    else if(a == 'B' || b == 'b')
    counterB++;
    else
    counter++;
}

One last thing, loops are expensive, don't do them twice when you can do them once. Depending on what you're trying to achieve, this could be done multiple ways.

Ok, so I stepped away from my computer for about 30 min and there were a lot of responses. Sorry for not coming back sooner. Yes, the intent is to count when 'A' or 'B' appears in the string (there are 4 different places this happens, each time appearing in a different position and incrementing by 7. I didn't post my first instance where this happened and I started at 0 because I didn't use two for loops as it asks for the position and 1,7,14etc. and not 2 and 3, 8, and 9 etc), and then find the average in regards to how many times 'B' appeared. I used two for loops because I'm not sure how to count in two places i.e. 1 and 2 and then increment by 7 and count in two places again, 8 and 9 in just one loop. If there's a better way to do this I'd love to know. I kind of just skimmed most of the responses. Let me look through them and see if I can't figure out my problem with you guys' suggestions.

---------- Post added 2012-11-20 at 03:58 AM ----------

Ok so, I forgot to tell you my problem. I'm getting the can't divide by 0 error. In the program it is impossible for the counts to be at 0 therefore I don't see a reason to check if they're not 0. To me this just means that the counters aren't incrementing correctly (staying at 0 because they didn't count up). Even if 'B' appears 0 times 'A' has to appear as those are the only two options. Help please

---------- Post added 2012-11-20 at 04:13 AM ----------

stop in the debugger, and look at the variables.
Or add a simple WriteLine to print out the values and look at them.

debugging by yourself is going to teach you a lot more than someone giving you the answer.

but just because I don't like people who don't debug, I'm going to tell you what I think the issue is, so you don't learn debugging skills:
When you divide int by int, you get int.
So: double sNAverage = ((counterB/(counterA + counterB))*100);
You're dividing counterB by counterA+counterB, so lets say counterB is 10, and counterA+counterB is 20, the answer will be 0, because 0.5 is not valid value for int.
So you gotta do something like:
double sNAverage = ((((double)counterB)/(counterA + counterB))*100);

or:
double sNAverage = counterB * 100.0 /(counterA + counterB);

yup. that solved my can't divide by 0 problem. however it still looks like my counters aren't incrementing. Still not sure why.

[Fortuitous]

Ok, that clears some things up. As far as the two loops being costly it is true but being a Java class, I'm not sure the professor cares about how efficient the code is. (Java at best is still very slow)

Now, what I think may be happening to cause your divide by zero is your indexes are off so where there should be an A or B in your string your position in the for loop is off by 1.. thus giving you no results. Start at 0

for(int str = 0; str < s.length(); str+=7)

Now, to make it a little more efficient, you could try implementing a way to compare two positions each iteration of the loop.

ch1 = Character.toUpperCase(s.charAt(str));
ch2 = Character.toUpperCase(s.charAt(str+1));

This will not protect against seg. faults (If ch1 is the end of the string.. ch2 might segfault)
I haven't programmed in Java for a while so I can't remember how it would handle array out of bounds.

[simplemind]

Ok, that clears some things up. As far as the two loops being costly it is true but being a Java class, I'm not sure the professor cares about how efficient the code is. (Java at best is still very slow)

Now, what I think may be happening to cause your divide by zero is your indexes are off so where there should be an A or B in your string your position in the for loop is off by 1.. thus giving you no results. Start at 0

for(int str = 0; str < s.length(); str+=7)

Now, to make it a little more efficient, you could try implementing a way to compare two positions each iteration of the loop.

ch1 = Character.toUpperCase(s.charAt(str));
ch2 = Character.toUpperCase(s.charAt(str+1));

This will not protect against seg. faults (If ch1 is the end of the string.. ch2 might segfault)
I haven't programmed in Java for a while so I can't remember how it would handle array out of bounds.

I mentioned this before, but I actually have a different method that starts at 0. There are actually 4 different percentage values I need. The first 1 starts at the first character in the string (I started at 0 here (I didn't include it in the thread because its just the same except I didn't have a second for loop)) then the next one starts at the second character in the string (1) and includes the third character (2) before incrementing. The string is actually 70 characters long. Thank you very much. the str+1 makes sense and I feel dumb for not thinking about it before... lol. That is definitely much better than 2 for loops. However my variables not incrementing problem still hasn't been solved

[stuck4cash]

Ok, that clears some things up. As far as the two loops being costly it is true but being a Java class, I'm not sure the professor cares about how efficient the code is. (Java at best is still very slow)

That's not true anymore. Java is comparable speed wise to C++ but also it's easier to craft more efficient algorithms quickly and therefore is often a better choice. It is definitely a memory hog though.

Now, what I think may be happening to cause your divide by zero is your indexes are off so where there should be an A or B in your string your position in the for loop is off by 1.. thus giving you no results. Start at 0

for(int str = 0; str < s.length(); str+=7)

Now, to make it a little more efficient, you could try implementing a way to compare two positions each iteration of the loop.

ch1 = Character.toUpperCase(s.charAt(str));
ch2 = Character.toUpperCase(s.charAt(str+1));

Edit: I just realised what you're doing here. You'd just stop looping at str.length()-1;. But based on what I think you're thinking, it's impossible for a array out of bounds since they're *always* coupled together and you're always looking for the first one. This seems like an odd problem though... definitely in the realms of homework question rather than real life problem.

This will not protect against seg. faults (If ch1 is the end of the string.. ch2 might segfault)
I haven't programmed in Java for a while so I can't remember how it would handle array out of bounds.

Array out of bounds throw an error. You could catch them but that's very very poor coding.

I'm still confused by the actual problem. Does the first A/B *always* appear at position 1 or 2 and either an A or B appears every 7 positions after that every time. Or does the first one occur any time after 0 and at least 7 positions after?

TBH, the 7 times after thing is irrelevant in that you could just test all and it'd work the same but I'm assuming it was intentionally added as it is a "homework" problem.

[Pejo]

Would you mind posting the whole code? Also, I do recommend running the debugger to see the value of CounterA and CounterB. It may be because of truncating due to your double average.

[simplemind]

That's not true anymore. Java is comparable speed wise to C++ but also it's easier to craft more efficient algorithms quickly and therefore is often a better choice. It is definitely a memory hog though.



Array out of bounds throw an error. You could catch them but that's very very poor coding.

I'm still confused by the actual problem. Does the first A/B *always* appear at position 1 or 2 and either an A or B appears every 7 positions after that every time. Or does the first one occur any time after 0 and at least 7 positions after?

TBH, the 7 times after thing is irrelevant in that you could just test all and it'd work the same but I'm assuming it was intentionally added as it is a "homework" problem.

Ok. So its given a string that's 70 characters long. Every character in the string is either an 'A' or a 'B'. There are 4 categories. These categories are broken up in that starting from the first character every seventh character corresponds with the first category. The second category (the only one I posted) starts at the second character, but it also includes the third character. Then every seventh character after the second character and every seventh character after the third character are in the second category. Third and 4th categories are just like the second however they start at 4 and 6.

[Fortuitous]

That's not true anymore. Java is comparable speed wise to C++ but also it's easier to craft more efficient algorithms quickly and therefore is often a better choice. It is definitely a memory hog though.

Array out of bounds throw an error. You could catch them but that's very very poor coding.

I just think Java has, though it has gotten significantly better, a disadvantage by default for its use of the virtual machine.

Also, to OP my apologies; I misunderstood the multiple counters thing.

I should also mention that I cannot recommend doing simply (str+1) because it is seriously begging for problems (as stuck4cash mentioned) but it was meant to show how you could manipulate multiple things into a single loop for efficiency.

[simplemind]

Would you mind posting the whole code? Also, I do recommend running the debugger to see the value of CounterA and CounterB. It may be because of truncating due to your double average.

I'd rather not because I don't want to be accused of cheating on the assignment. I'd rather my teacher not search for my code only to find this thread. Also its a pretty big program this is just one small problem I'm having with it. And after the guy who hates me for asking for help fixed my not divisable by 0 error I ran the debugger and the values I need are returning as 0. Clearly my counters aren't incrementing.

[stuck4cash]

I just think Java has, though it has gotten significantly better, a disadvantage by default for its use of the virtual machine.

The VM is quite light weight and other than memory, doesn't scale up with the code. I don't see any reason why it continues to get this bad wrap about speed, it hasn't been the case since the 90s. It definitely uses more memory when compared to a well written c++ program (of course if they don't clean up properly in that then you can end up with some mess.[/quote]

Also, to OP my apologies; I misunderstood the multiple counters thing.

I should also mention that I cannot recommend doing simply (str+1) because it is seriously begging for problems (as stuck4cash mentioned) but it was meant to show how you could manipulate multiple things into a single loop for efficiency.[/QUOTE]

I think I see what you're trying to do here now.

for(int i = 1; i < line.length(); i+=7){
    c1 = line.charAt(i);
    c2 = 'x'; //dummy char in case it falls off the end of the string
    //not sure that this would happen as since 1 + 7*10 = 71 so off the string anyway.
    if(i < line.length()-1)
        c2 = line.charAt(i+1);
    if(c1 == 'a' || c1 == 'A' || c2 == 'a' || c2 == 'A')
        counterA++;
    else if(c1 == 'b' || c1 == 'B' || c2 == 'b' || c2 == 'B')
        counterB++;
    else
        //not necessary really
        counter++;
}
//do other stuff as necessary.

To be honest, I think the comments covers the unnecessary bits and you could just change the out of bounds protection stuff but yeah. Note: cleaned up to reduce confusion and stick to accepted java standards a bit better.

P.S. A few other stupid questions, why is it a static method? and why do you pass in the counter numbers? Wouldn't you want your counters starting at 0 for the beginning of the string each time?

Edit: One thing that I'm not sure on is the non-brackets else's when there's a comment. Obviously no brackets is next line only but I think that's after you remove white space/comments. Someone should clarify that for me.

---------- Post added 2012-11-20 at 04:25 PM ----------

System.out.println(s);

Put that in directly after s = read.next(). Then add some:

System.out.println("Found an A: " + line.charAt(i));

bits inside your if statements to see where it's at.

If you're still having no luck, check your for loop to see what character it is jumping to next:

System.out.println(i);

You'll find some clues in there I'm sure.

I know it's unlikely as well but put some counterA = 0; and counterB = 0; at the top. I know it's unlikely but in theory they could be going into the function with values of -7 (assuming there's 7 As in the string).

[simplemind]

Static method because we haven't learned the other types yet. The counter variables are passed in because I actually use them in more than one method (as mentioned before I use 4 different methods to do the same thing in different places in the string) they are initialized outside of this method to be 0. Thx for the tips on checking what my code is doing. I'm thinking that the problem may lie somewhere in the code outside of what I posted, so your tips will be very helpful as to helping me figure it out. I'll look at it again tomorrow and depending on my mood and how many more hours I spend trying to figure it out I may just post my whole code here. Thx everyone who has helped out so far. Goodnight

[Deleted]

Don't pass the counters in, that's just bad as they are passed by value so they will not be modified outside this method, also if you have repeated code you are doing something wrong.

[simplemind]

Don't pass the counters in, that's just bad as they are passed by value so they will not be modified outside this method, also if you have repeated code you are doing something wrong.

I don't need them to be modified outside of the method. I need them to start at 0 when I use them in each of my 4 methods. The only thing I need is the average that they appear in in each part of the string. The code isn't technically repeated. There are 4 different sections of the string that I'm checking and they're not in sequential order (1,2,3). So I use each method to check each different area of the string. Even though the methods are practically the same except one difference in each. They aren't really the same.

[Deleted]

Ok. So its given a string that's 70 characters long. Every character in the string is either an 'A' or a 'B'. There are 4 categories. These categories are broken up in that starting from the first character every seventh character corresponds with the first category. The second category (the only one I posted) starts at the second character, but it also includes the third character. Then every seventh character after the second character and every seventh character after the third character are in the second category. Third and 4th categories are just like the second however they start at 4 and 6.

Global A,B Counter did nothing
Counter did even less since every character is A/B

  public static int sN(Scanner read)
    public static int sN(Scanner read)
    {
        char currentCharacter = '0';
		int ACount = 0;
		int BCount = 0;
        while(read.hasNextLine())
        {
           String s = read.next();

           for(int str = 1; str < s.length(); str+=7)
           {
			   ACount = ACount + charComparision(s.charAt(str), 'a');
			   BCount = BCount + charComparision(s.charAt(str), 'b');
           }
           for(int str = 2; str < s.length(); str+=7)
           {
			   ACount = ACount + charComparision(s.charAt(str), 'a');
			   BCount = BCount + charComparision(s.charAt(str), 'b');
           }
        }
		
        double sNAverage = ((counterB/(counterA + counterB))*100);
        int sNPercent = (int)(sNAverage + 0.5);
        return sNPercent; 
    }
	
	public static int charComparision(char src, char cmp){
		char tmpsrc = Character.toUpperCase(src);
		char tmpcmp = Character.toUpperCase(cmp);
		if (src == cmp) return 1;
		return 0;	
	}

You could also do the whole Count for all 4 Categories in 1 loop

[simplemind]

Ok. I figured it out. It was somewhere else in the program. Also, I'm going to delete my code so that I can't be accused of cheating. Thx everyone for all of the help!.