3 ms·

Non-programmer here, can someone explain what is wrong with that code?

cmptrnerd6 3y ago
Edge cases. If d is very large then 2*d might be larger than what can be computed. Also I think the code could just multiply d by -1 which would "reverse the sign". Even that has some edge cases that should be handled around the maximum value of an integer. But the main thing to me is that as written this code is confusing and overcomplicating the easy part of the problem.
jph 3y ago
Yes. Flipping the sign of a number can typically be done in one line as `d = -d`. Or if that syntax isn't available in that programming language, then `d = 0 - d` or `d = -1 * d`. What's wrong with code above is 1) it does a conditional branch (meaning if.. then... else...) which is unneeded. 2) The "then clause" does an absolute value which is a waste of a function call. 3) The "else clause" does `d = d - (2 * d)` which is a waste of a multiplication 4) possibly with an overflow risk and/or floating point estimation risk. That said, it's admittedly possible that the programming language above, or the number representation in that code above, have some highly atypical needs that we here on HN don't understand.
- nerdawson 3y ago
  Maybe they were measuring programmer output in KLOCs.
- tauchunfall 3y ago
  They had teams with software engineers with different levels of competence and experience [1]. There were good, mediocre, and members that were not so capable. Which is true for many teams. When at least the good members have the time and review the code of the others, than this code would probably not go into production. Even when these code snippets are cherry-picked they tell enough about their review practices. >have some highly atypical needs that we here on HN don't understand. Somebody else checked in another thread. You can use unary minus in VBA. [1] >One member of the development team, David McDonnell, who had worked on the Epos system side of the project, told the inquiry that “of eight [people] in the development team, two were very good, another two were mediocre but we could work with them, and then there were probably three or four who just weren’t up to it and weren’t capable of producing professional code”.
woodrowbarlow 3y ago
it appears correct but over-complicated -- it should be a single line of code. the over-complicated solution may have underflow/overflow problems with very large numbers.

Edge cases. If d is very large then 2*d might be larger than what can be computed. Also I think the code could just multiply d by -1 which would "reverse the sign". Even that has some edge cases that should be handled around the maximum value of an integer. But the main thing to me is that as written this code is confusing and overcomplicating the easy part of the problem.

jph 3y ago

Yes. Flipping the sign of a number can typically be done in one line as `d = -d`. Or if that syntax isn't available in that programming language, then `d = 0 - d` or `d = -1 * d`. What's wrong with code above is 1) it does a conditional branch (meaning if.. then... else...) which is unneeded. 2) The "then clause" does an absolute value which is a waste of a function call. 3) The "else clause" does `d = d - (2 * d)` which is a waste of a multiplication 4) possibly with an overflow risk and/or floating point estimation risk. That said, it's admittedly possible that the programming language above, or the number representation in that code above, have some highly atypical needs that we here on HN don't understand.

nerdawson 3y ago
Maybe they were measuring programmer output in KLOCs.
tauchunfall 3y ago
They had teams with software engineers with different levels of competence and experience [1]. There were good, mediocre, and members that were not so capable. Which is true for many teams. When at least the good members have the time and review the code of the others, than this code would probably not go into production. Even when these code snippets are cherry-picked they tell enough about their review practices. >have some highly atypical needs that we here on HN don't understand. Somebody else checked in another thread. You can use unary minus in VBA. [1] >One member of the development team, David McDonnell, who had worked on the Epos system side of the project, told the inquiry that “of eight [people] in the development team, two were very good, another two were mediocre but we could work with them, and then there were probably three or four who just weren’t up to it and weren’t capable of producing professional code”.

woodrowbarlow 3y ago

it appears correct but over-complicated -- it should be a single line of code. the over-complicated solution may have underflow/overflow problems with very large numbers.