Skip to content

Decimal Expansion#787

Merged
raklaptudirm merged 8 commits into
TheAlgorithms:masterfrom
lvlte:Algorithms/DecimalExpansion
Apr 2, 2022
Merged

Decimal Expansion#787
raklaptudirm merged 8 commits into
TheAlgorithms:masterfrom
lvlte:Algorithms/DecimalExpansion

Conversation

@lvlte

@lvlte lvlte commented Oct 14, 2021

Copy link
Copy Markdown
Contributor

Adding algorithm for representing the decimal expansion of a given fraction a/b using euclidean division. Optionally, the representation can be in binary, ternary, octal, any base from 2 to 10.

Code is compliant with Javascript standard and covered by Jest tests.

@lvlte lvlte requested a review from raklaptudirm as a code owner October 14, 2021 11:39
@raklaptudirm raklaptudirm added algorithm Adds or improves an algorithm feature Adds a new feature Reviewed labels Oct 16, 2021

@raklaptudirm raklaptudirm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I suggestion: Since you are using a fixed size array as the result, wouldn't is be better to use an object:

{
  integer: 3,
  decimal: [1, 4, 1, 5],
  indexOf: undefined
}

@lvlte

lvlte commented Oct 16, 2021

Copy link
Copy Markdown
Contributor Author

In this particular case, I prefer array vs object, because it left the freedom to name variables when doing destructuring assignments, which is probably better here because this function can return fraction decimals by default, but also bits, trits, octals, etc.

Using an object, variables identitifers will always be restricted to the object properties, regardless of the base, which might be more conducive to confusion if the base used is not 10 :
const {integer, decimal, indexOf} = decExp(a, b, 8);

Using an array leaves more freedom :
const [integer, decimals, index] = decExp(a, b);
const [integer, octals, index] = decExp(a, b, 8);
const [int, trits, i] = decExp(a, b, 3)

This is like hard vs soft code for variable naming/assignment, it encourages the developer to define (meaningful) variables by himself, depending on the situation (btw I realized I refer to decimals regardless of the base in the tests). But if he does not, I agree it is not an advantage at all, eg. in this case :

decExpArray = decExp(a, b);
// doing things with decExpArray[0]
// doing things with decExpArray[1]
// doing things with decExpArray[2] ... 

What's your opinion ?

@raklaptudirm

Copy link
Copy Markdown
Member

I am ok with it, I just feel using an object would be more self explanatory.

@raklaptudirm raklaptudirm merged commit 2be0fdc into TheAlgorithms:master Apr 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

algorithm Adds or improves an algorithm feature Adds a new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants