bugs in java code after converting from C#

Question

here is a function prints repeating int in a array. in c#:

int [] ReturnDups(int[] a)
{
   int repeats = 0;
   Dictionary<int, bool> hash = new Dictionary<int>();
   for(int i = 0; i < a.Length i++)
   {
       bool repeatSeen;
       if (hash.TryGetValue(a[i], out repeatSeen))
       {
           if (!repeatSeen)
           {
               hash[a[i]] = true;
               repeats ++;
           }
       }
       else
       {
           hash[a[i]] = false;
       }
   }

   int[] result = new int[repeats];
   int current = 0;
   if (repeats > 0)
   {
     foreach(KeyValuePair<int,bool> p in hash)
     {
         if(p.Value)
         {
             result[current++] = p.Key;
         }
     }
   }    
   return result;
}

now converted to JAVA by Tangible software's tool. in java:

private int[] ReturnDups(int[] a)
{
   int repeats = 0;
   java.util.HashMap<Integer, Boolean> hash = new java.util.HashMap<Integer>();
   for (int i = 0; i < a.length i++)
   {
       boolean repeatSeen = false;
       if (hash.containsKey(a[i]) ? (repeatSeen = hash.get(a[i])) == repeatSeen : false)
       {
           if (!repeatSeen)
           {
               hash.put(a[i], true);
               repeats++;
           }
       }
       else
       {
           hash.put(a[i], false);
       }
   }

   int[] result = new int[repeats];
   int current = 0;
   if (repeats > 0)
   {
     for (java.util.Map.Entry<Integer,Boolean> p : hash.entrySet())
     {
         if (p.getValue())
         {
             result[current++] = p.getKey();
         }
     }
   }

   return result;
}

but findbug find this line of code as bugs. and it looks very odd to me too.

if (hash.containsKey(a[i]) ? (repeatSeen = hash.get(a[i])) == repeatSeen : false)

can someone pls explain to me what this line does and how do i write it in java properly? thanks

you are missing `;` semicolon in your for loop. `for (int i = 0; i < a.length i++)` — Tun Zarni Kyaw, Jan 03 '14 at 03:03
"Self comparison of repeatSeen with itself" - findRepeat.javaFindBugs Problem (Scariest) — user3143258, Jan 03 '14 at 03:03
`Convert C# to java` - C# (above 2.0) cannot be converted to java. java is a crappy language that is stuck in the 90's, while C# is constantly evolving. Try converting C# 5 code full of LINQ, delegates, properties, events, type inference, using statements, and `async/await` to java. — Federico Berasategui, Jan 03 '14 at 03:26
If you need to support non-Windows platforms, get Xamarin instead. It is far cheaper than the medical bills you will have to pay for all C# developers suffering java-based cancer in the balls — Federico Berasategui, Jan 03 '14 at 03:27
For example, all that poorly written C# code can be rewritten into this: `var dups = a.GroupBy(x => x) .Where(x => x.Count() > 1) .Select(x => x.First());` single statement which finds duplicates in an `IEnumerable` — Federico Berasategui, Jan 03 '14 at 03:39

score 1 · Accepted Answer · answered Jan 03 '14 at 03:04

1

You have overcomplicated the code for TryGetValue - this simple translation should work:

if ( hash.containsKey(a[i]) ) {
    if (!hash.get(a[i])) {
        hash.put(a[i], true);
    }
} else {
    hash.put(a[i], false);
}

C# has a way to get the value and a flag that tells you if the value has been found in a single call; Java does not have a similar API, because it lacks an ability to pass variables by reference.

answered Jan 03 '14 at 03:04

Sergey Kalinichenko

714,442
84
1,110
1,523

thanks, it's working now. i still need to add repeats++ in the loop because after hashmap is created, i need to iterate through hashmap to print out repeated int. thanks a lot. – user3143258 Jan 03 '14 at 03:28

score 0 · Answer 2 · answered Jan 03 '14 at 03:07

Do not directly convert C# implementation. assign repeatSeen value only if the id is there.

if (hash.containsKey(a[i]))
           {
               repeatSeen = hash.get(a[i]).equals(repeatSeen)
               if (!repeatSeen)
               {
                   hash.put(a[i], true);
                   repeats++;
               }
           }

score 0 · Answer 3 · edited May 23 '17 at 12:11

To answer the actual question that was asked:

if (hash.containsKey(a[i]) ? (repeatSeen = hash.get(a[i])) == repeatSeen : false)

is indeed syntactically wrong. I haven't looked at the rest of the code, but having written parsers/code-generators in my time I'm guessing it was supposed to be

if (hash.containsKey(a[i]) ? (repeatSeen = hash.get(a[i])) == repeatSeen) : false)

It's gratuitously ugly -- which often happens with code generators, especially ones without an optimizing pass -- but it's syntactically correct. Let's see if it actually does have a well-defined meaning.

CAVEAT: I haven't crosschecked this by running it -- if someone spots an error, please tell me!

First off, x?y:z is indeed a ternary operator, which Java inherited from C via C++. It's an if-then-else expression -- if x is true it has the value y, whereas if x is false it has the value z. So this one-liner means the same thing as:

boolean implied;
if (hash.containsKey(a[i]) then 
  implied = (repeatSeen = hash.get(a[i])) == repeatSeen);
else
  implied = false;
if(implied)

... and so on.

Now, the remaining bit of ugliness is the second half of that and-expression. I don't know if you're familiar with the use of = (assignment) as an expression operator; its value as an operator is the same value being assigned to the variable. That's mostly intended to let you do things like a=b=0;, but it can also be used to set variables "in passing" in the middle of an expression. Hardcore C hackers do some very clever, and ugly, things with it (he says, being one)... and here's it's being used to get the value from the hashtable, assign it to repeatSeen, and then -- via the == -- test that same value against repeatSeen.

Now the question is, what order are the two arguments of == evaluated in? If the left side is evaluated first, the == must always be true because the assignment will occur before the right-hand side retrieves the value. If the right side is evaluated first, we'd be comparing the new value against the previous value, in an very non-obvious way.

Well, in fact, there's another StackOverflow entry which addresses that question:

What are the rules for evaluation order in Java?

According to that, the rule for Java is that the left argument of an operator is always evaluated before the right argument. So the first case applies, the == always returns true.

Rewriting our translation one more time to reflect that, it turns into

boolean implied;
if (hash.containsKey(a[i]) then 
{
  repeatSeen = hash.get(a[i]));
  implied = true;
}
else
  implied = false;
if(implied)

Which could be further rewritten as

if (hash.containsKey(a[i]) then 
{
  repeatSeen = hash.get(a[i]));
  // and go on to do whatever else was in the body of the original if statement

"If that's what they meant, why didn't they just write it that way?" ... As I say, I've written code generators, and in many cases the easiest thing to do is just make sure all the fragments you're writing are individually correct for what they're trying to do and not worry about whether they at all resemble what a human would have written do do the same thing. In particular, it's tempting to generate code according to templates which allow for cases you may not actually use, rather than trying to recognize the simpler situation and generate code differently.

I'm guessing that the compiler was drawing in and translating bits of computation as it realized it needed them, and that this created the odd nesting as it started the if, then realized it needed a conditional assignment to repeatSeen, and for whatever reason tried to make that happen in the if's test rather than in its body. Believe me, I've seen worse kluging from code generators.

bugs in java code after converting from C#

3 Answers3