Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

optimization of the adjustNumber function #107

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Akvadevka
Copy link

@Akvadevka Akvadevka commented Oct 25, 2024

I refactored adjustNumber to enhance readability and simplify the logic, reducing the number of checks and iterations over the array.


PR-Codex overview

This PR focuses on improving the adjustNumber function in the eo2js-runtime/src/runtime/bytes-of.js file, specifically simplifying its logic and enhancing readability.

Detailed summary

  • Removed unnecessary check for bytes.length === 8 and replaced it with bytes.length !== 8 for early return.
  • Simplified the mapping of bytes by using a ternary operator instead of an if-else structure.
  • Cleaned up the formatting of error messages.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

I refactored adjustNumber to enhance readability and simplify the logic, reducing the number of checks and iterations over the array.
@yegor256
Copy link
Member

@maxonfjvipon please, review this one

@maxonfjvipon
Copy link
Member

@Akvadevka what ticket your pull request is connected to? What problem did you try to solve?

@Akvadevka
Copy link
Author

@maxonfjvipon no ticket. I wrote my purposes in the description.

@@ -24,7 +24,7 @@ const hexToInt = function(bytes) {
byte = parseInt(hex, 16)
} else {
throw new Error(
`Wrong format of element ${hex} in byte array ${bytes}\nShould be either integer of hexadecimal starting with '0x'`
`Wrong format of element ${hex} in byte array ${bytes}\nShould be either integer of hexadecimal starting with '0x'`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Akvadevka there's something wrong with indentation

@@ -38,19 +38,16 @@ const hexToInt = function(bytes) {
* @return {Array.<Number>} - Adjusted byte array
*/
const adjustNumber = function(bytes) {
if (bytes.length === 8) {
const number = bytesOf(
if (bytes.length !== 8) return bytes;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Akvadevka we're trying to avoid many return statements, here's why

).asBytes();

return bytes.map((byte, index) =>
Math.abs(byte - number[index]) === 256 ? byte - 256 : byte
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Akvadevka I don't see the purpose of the change. Replacing if with ternary operator didn't make the code more readable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants