The output of this loop should be less than 1, but I get very large numbers

Question

This program is supposed to count the number of characters "c" and "g" in genes in the DNA string and then take that number and divide it by the length of each gene. The number of cs and gs is always < gene.length(), therefore the output should be something like 0.65555, 0.35657 etc, but I get large numbers like 141, etc. Not sure what is wrong with this loop.

public void testfile(){
    String dnaTest = "aaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaacccttaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctaaccctcacccttctaact";


    int counter = 0;
        for(String gene : s1.data()) {

            for (char cORg : gene.toCharArray()) {
                if ((cORg == 'c') || (cORg == 'g')) {
                    counter ++;
                 }

              System.out.print(gene +" ");

            }
            float cgRatioGenes =  counter/gene.length();
            System.out.println("cgRatio:  " + cgRatioGenes);
        }
    }
}

If you spot the error, let me know. Thanks!

EDIT

Even without the parentesis at the end of the DNA string and with the closing bracket, the loop was not producing the results I expected. Therefore, it is not off topic.

q1: what is "s1.data()"? q2: if you divide an int by an int the result is int. please cast at least one of them to float before division. — Christoph-Tobias Schenke, May 09 '16 at 15:09
Did you perhaps mean to compute and print `cgRatioGenes` within the outermost `for` loop? — Bethany Louise, May 09 '16 at 15:10
Possible duplicate of [Integer division: How do you produce a double?](http://stackoverflow.com/questions/3144610/integer-division-how-do-you-produce-a-double) — Tom, May 09 '16 at 15:14
Won't compile because of a `)` at the end of the `dnaTest` line. What is the purpose of that `dnaTest` anyway? It is unused by displayed code. — Andreas, May 09 '16 at 15:20
@Andreas it's a typo. There shoulddn't be a quote at the and. I decided not to remove the DNA string, because some people who view this question may not know what it looks like. I figured it'd be easier to imagine what I was talking about. — Ekaterina1234, May 09 '16 at 15:34
The loop would't work even without the ')' at the end of the DNA string. It is not off topic. @Chloe. The answer provided by resueman fixed the error. — Ekaterina1234, May 10 '16 at 10:40

score 4 · Accepted Answer · edited May 23 '17 at 12:23

4

Two problems:

First, you never reinitialize counter when you start the loop again. Move that declaration inside the loop so that each repetition starts with a counter of zero.

If you make that change, all your results will be zero though, because you're diving two integers, which will truncate the results. Cast one to float, so that it keeps the decimal part. See this question for more information on the problem

for(String gene : s1.data()) {
    int counter = 0; //Moved inside the for loop, so that it always starts at 0
    for (char cORg : gene.toCharArray()) {
        if ((cORg == 'c') || (cORg == 'g')) {
            counter ++;
        }

        System.out.print(gene +" ");
    }

    //Floating point division, instead of integer division
    float cgRatioGenes =  ((float)counter)/gene.length(); 
    System.out.println("cgRatio:  " + cgRatioGenes);
}

edited May 23 '17 at 12:23

Community

1
1

answered May 09 '16 at 15:11

resueman

10,572
10
31
45

1

And *maybe* OP got confused about code because indentations were misleading. Sure confused me for a second, since a close-brace (`}`) was missing. – Andreas May 09 '16 at 15:21
Thanks! It works as intended now. – Ekaterina1234 May 09 '16 at 15:26
@Ekaterina1234 I know it might just be a result of copying your code into the question, but as [Andreas](http://stackoverflow.com/users/5221149/andreas) points out, make sure your indentation is correct; otherwise it's very easy to get confused. – resueman May 09 '16 at 15:32
@resueman apologies for that, I had some other stuff in the original code, that was not relevant tho the question, so I deleted it, and probably deleted the brace as well. Next time I'll indent the code properly, so that it's easier to read. Thanks for taking the time to answer my question! I appreciate it! – Ekaterina1234 May 10 '16 at 10:47

score 3 · Answer 2 · answered May 09 '16 at 15:10

One potential problem is here

float cgRatioGenes = counter/gene.length();

As gene.length() is an integer value the ratio is not computed correctly. Instead, you should cast one of them to float like this

float cgRatioGenes = ((float)counter)/gene.length();

In addition, the counter variable should probably be initialized to zero for each gene (unless you want to count the c/g values over all genes).

This probably does not explain the behavior you are observing, but it is not possible to figure it out unless a complete working example is given.

score 0 · Answer 3 · answered May 09 '16 at 15:11

It's a little unclear what the exact intent of this code is, but my guess is that you're using one int counter for every gene in s1.data(). I assume you want to count the number of valid characters per gene, not in the entire pool.

If you do want to count for the entire pool, the problem is that you're peforming gene.length outside of the for, which should honestly throw a compiler error unless you have a gene defined somewhere else as well.

Additionally, you're dividing two ints for your answer, which will yield an int. Case one of your variables to float to get a decimal answer.

The output of this loop should be less than 1, but I get very large numbers

3 Answers3