Skip to main content
Code Review

Return to Answer

added 159 characters in body
Source Link
David Foerster
  • 1.7k
  • 10
  • 20

Documentation

I took me a while to understand and research the algorithm and the purpose of the function parameters. It would be better to write some Javadoc for reviewers and future readers (which may include yourself).

Identifiers

You should use variable and function identifiers that explain their purpose (barring obvious cases like i for a loop iterator).

  • strtof is a function defined since C89 and does something very different from your function; that's not a good name here. What about FPxToFloat because the function converts data in a variant of the FP* formats (x instead of *) to a float?

  • e is the number of bits in the integral part of the number. How about integerBits instead?

  • total – My first thought was "total of what?". I usually go for a name like result in these cases since it holds the function result (or an intermediate result on the way to the final result) which should be well documented somewhere nearby.

Limitations

  • Why is size restricted to ≤ 4 bytes and integerBits restricted to ≤ 8. You say you want a more generic solution. I don't see a reason why size can't be 17 and integerBits 49.

  • Currently the encoded number needs to start at offset 0 inside the bytes array. Since Java doesn't have C's pointer arithmetic it would be useful to offer an offset parameter like the read and write functions in java.io.

    You can still offer a convenience function with less boiler plate:

     public static float FPxToFloat(byte[] bytes, int integerBits) {
     return FPxToFloat(bytes, 0, bytes.length, integerBits);
     }
    

Complexity

Your implementation seems overly complicated and suffers from a number of unnecessary limitations (see first point in the section above). What about this:

double result = 0;
int exponent = integerBits - Byte.SIZE;
for (; offset < size; offset++, exponent -= Byte.SIZE) {
 result += Math.scalb(bytes[offset] & 0xFF, exponent);
}
return result;

scalb avoids multiplicative floating-point operations and overflow of integers that shall later be converted to floating-point. The only optimization I can think of is to work on long instead of byte if possible to use the available processor word size better (to achieve that one can wrap byte[] inside a java.nio.ByteBuffer and use a LongBuffer view onto it).

Full program (Ideone is currently acting up here so a Gist it has to be for now.)

Documentation

I took me a while to understand and research the algorithm and the purpose of the function parameters. It would be better to write some Javadoc for reviewers and future readers (which may include yourself).

Identifiers

You should use variable and function identifiers that explain their purpose (barring obvious cases like i for a loop iterator).

  • strtof is a function defined since C89 and does something very different from your function; that's not a good name here. What about FPxToFloat because the function converts data in a variant of the FP* formats (x instead of *) to a float?

  • e is the number of bits in the integral part of the number. How about integerBits instead?

  • total – My first thought was "total of what?". I usually go for a name like result in these cases since it holds the function result (or an intermediate result on the way to the final result) which should be well documented somewhere nearby.

Limitations

  • Why is size restricted to ≤ 4 bytes and integerBits restricted to ≤ 8. You say you want a more generic solution. I don't see a reason why size can't be 17 and integerBits 49.

  • Currently the encoded number needs to start at offset 0 inside the bytes array. Since Java doesn't have C's pointer arithmetic it would be useful to offer an offset parameter like the read and write functions in java.io.

    You can still offer a convenience function with less boiler plate:

     public static float FPxToFloat(byte[] bytes, int integerBits) {
     return FPxToFloat(bytes, 0, bytes.length, integerBits);
     }
    

Complexity

Your implementation seems overly complicated and suffers from a number of unnecessary limitations (see first point in the section above). What about this:

double result = 0;
int exponent = integerBits - Byte.SIZE;
for (; offset < size; offset++, exponent -= Byte.SIZE) {
 result += Math.scalb(bytes[offset] & 0xFF, exponent);
}
return result;

scalb avoids multiplicative floating-point operations and overflow of integers that shall later be converted to floating-point. The only optimization I can think of is to work on long instead of byte if possible to use the available processor word size better (to achieve that one can wrap byte[] inside a java.nio.ByteBuffer and use a LongBuffer view onto it).

Documentation

I took me a while to understand and research the algorithm and the purpose of the function parameters. It would be better to write some Javadoc for reviewers and future readers (which may include yourself).

Identifiers

You should use variable and function identifiers that explain their purpose (barring obvious cases like i for a loop iterator).

  • strtof is a function defined since C89 and does something very different from your function; that's not a good name here. What about FPxToFloat because the function converts data in a variant of the FP* formats (x instead of *) to a float?

  • e is the number of bits in the integral part of the number. How about integerBits instead?

  • total – My first thought was "total of what?". I usually go for a name like result in these cases since it holds the function result (or an intermediate result on the way to the final result) which should be well documented somewhere nearby.

Limitations

  • Why is size restricted to ≤ 4 bytes and integerBits restricted to ≤ 8. You say you want a more generic solution. I don't see a reason why size can't be 17 and integerBits 49.

  • Currently the encoded number needs to start at offset 0 inside the bytes array. Since Java doesn't have C's pointer arithmetic it would be useful to offer an offset parameter like the read and write functions in java.io.

    You can still offer a convenience function with less boiler plate:

     public static float FPxToFloat(byte[] bytes, int integerBits) {
     return FPxToFloat(bytes, 0, bytes.length, integerBits);
     }
    

Complexity

Your implementation seems overly complicated and suffers from a number of unnecessary limitations (see first point in the section above). What about this:

double result = 0;
int exponent = integerBits - Byte.SIZE;
for (; offset < size; offset++, exponent -= Byte.SIZE) {
 result += Math.scalb(bytes[offset] & 0xFF, exponent);
}
return result;

scalb avoids multiplicative floating-point operations and overflow of integers that shall later be converted to floating-point. The only optimization I can think of is to work on long instead of byte if possible to use the available processor word size better (to achieve that one can wrap byte[] inside a java.nio.ByteBuffer and use a LongBuffer view onto it).

Full program (Ideone is currently acting up here so a Gist it has to be for now.)

added 1748 characters in body
Source Link
David Foerster
  • 1.7k
  • 10
  • 20

Documentation

I don't fullytook me a while to understand your proposedand research the algorithm yet so I focus onand the part that I do understandpurpose of the function parameters. It would be better to write some Javadoc for reviewers and future readers (which may include yourself).

Identifiers

You should use variable and function identifiers that explain their purpose (barring obvious cases like i for a loop iterator).

  • strtof is a function defined since C89 and does something very different from your function; that's not a good name here. What about FPExToFloat because the function converts data in a variant of the FPE* formats (x instead of *) to a float?

    strtof is a function defined since C89 and does something very different from your function; that's not a good name here. What about FPxToFloat because the function converts data in a variant of the FP* formats (x instead of *) to a float?

  • e is the number of bits in the integral part of the number. How about integerBits instead?

  • total – My first thought was "total of what?". I usually go for a name like result in these cases since it holds the function result (or an intermediate result on the way to the final result) which should be well documented somewhere nearby.

Limitations

  • Why is size restricted to ≤ 4 bytes and integerBits restricted to ≤ 8. You say you want a more generic solution. I don't see a reason why size can't be 17 and integerBits 49.

  • Currently the encoded number needs to start at offset 0 inside the bytes array. Since Java doesn't have C's pointer arithmetic it would be useful to offer an offset parameter like the read and write functions in java.io.

    You can still offer a convenience function with less boiler plate:

     public static float FPxToFloat(byte[] bytes, int integerBits) {
     return FPxToFloat(bytes, 0, bytes.length, integerBits);
     }
    

Complexity

Your algorithm runs into issues if 2^e >implementation seems overly complicated and suffers from a number of unnecessary limitations (see first point in the section above). What about this:

double result = 0;
int exponent = integerBits - Byte.SIZE;
for (; offset < size; offset++, exponent -= Byte.SIZE) {
 result += Math.scalb(bytes[offset] & 0xFF, exponent);
}
return result;

Integer.MAX_VALUEscalb avoids multiplicative floating-point operations and overflow of integers that shall later be converted to floating-point. I will draft a way around this onceThe only optimization I understandcan think of is to work on long instead of byte if possible to use the algorithmavailable processor word size better (to achieve that one can wrap byte[] inside a java.nio.ByteBuffer and use a LongBuffer view onto it).

I don't fully understand your proposed algorithm yet so I focus on the part that I do understand.

Identifiers

You should use variable and function identifiers that explain their purpose (barring obvious cases like i for a loop iterator).

  • strtof is a function defined since C89 and does something very different from your function; that's not a good name here. What about FPExToFloat because the function converts data in a variant of the FPE* formats (x instead of *) to a float?

Limitations

Your algorithm runs into issues if 2^e > Integer.MAX_VALUE. I will draft a way around this once I understand the algorithm better.

Documentation

I took me a while to understand and research the algorithm and the purpose of the function parameters. It would be better to write some Javadoc for reviewers and future readers (which may include yourself).

Identifiers

You should use variable and function identifiers that explain their purpose (barring obvious cases like i for a loop iterator).

  • strtof is a function defined since C89 and does something very different from your function; that's not a good name here. What about FPxToFloat because the function converts data in a variant of the FP* formats (x instead of *) to a float?

  • e is the number of bits in the integral part of the number. How about integerBits instead?

  • total – My first thought was "total of what?". I usually go for a name like result in these cases since it holds the function result (or an intermediate result on the way to the final result) which should be well documented somewhere nearby.

Limitations

  • Why is size restricted to ≤ 4 bytes and integerBits restricted to ≤ 8. You say you want a more generic solution. I don't see a reason why size can't be 17 and integerBits 49.

  • Currently the encoded number needs to start at offset 0 inside the bytes array. Since Java doesn't have C's pointer arithmetic it would be useful to offer an offset parameter like the read and write functions in java.io.

    You can still offer a convenience function with less boiler plate:

     public static float FPxToFloat(byte[] bytes, int integerBits) {
     return FPxToFloat(bytes, 0, bytes.length, integerBits);
     }
    

Complexity

Your implementation seems overly complicated and suffers from a number of unnecessary limitations (see first point in the section above). What about this:

double result = 0;
int exponent = integerBits - Byte.SIZE;
for (; offset < size; offset++, exponent -= Byte.SIZE) {
 result += Math.scalb(bytes[offset] & 0xFF, exponent);
}
return result;

scalb avoids multiplicative floating-point operations and overflow of integers that shall later be converted to floating-point. The only optimization I can think of is to work on long instead of byte if possible to use the available processor word size better (to achieve that one can wrap byte[] inside a java.nio.ByteBuffer and use a LongBuffer view onto it).

deleted 2 characters in body
Source Link
David Foerster
  • 1.7k
  • 10
  • 20

I don't fully understand your proposed algorithm yet so I focus on the part that I do understand.

Identifiers

You should use variable and function identifiers that explain their purpose (barring obvious cases like i for a loop iterator).

  • strtof is a function defined since C89 and does something very different from your function; that's not a good name here. What about fpextofFPExToFloat because the function converts data in a variant of the FPE* formats (x instead of *) to a float (f)?

Limitations

Your algorithm runs into issues if 2^e > Integer.MAX_VALUE. I will draft a way around this once I understand the algorithm better.

I don't fully understand your proposed algorithm yet so I focus on the part that I do understand.

Identifiers

You should use variable and function identifiers that explain their purpose (barring obvious cases like i for a loop iterator).

  • strtof is a function defined since C89 and does something very different from your function; that's not a good name here. What about fpextof because the function converts data in a variant of the FPE* formats (x instead of *) to a float (f)?

Limitations

Your algorithm runs into issues if 2^e > Integer.MAX_VALUE. I will draft a way around this once I understand the algorithm better.

I don't fully understand your proposed algorithm yet so I focus on the part that I do understand.

Identifiers

You should use variable and function identifiers that explain their purpose (barring obvious cases like i for a loop iterator).

  • strtof is a function defined since C89 and does something very different from your function; that's not a good name here. What about FPExToFloat because the function converts data in a variant of the FPE* formats (x instead of *) to a float?

Limitations

Your algorithm runs into issues if 2^e > Integer.MAX_VALUE. I will draft a way around this once I understand the algorithm better.

Source Link
David Foerster
  • 1.7k
  • 10
  • 20
Loading
lang-java

AltStyle によって変換されたページ (->オリジナル) /