[feat/fix/docs]: Improved on conversion code , length variable , asserts added #840

lazy-dude · 2021-07-08T11:33:28Z

Description of Change

References

Checklist

Added description of change
Added file name matches File name guidelines
Added tests and example, test must pass
Relevant documentation/comments is changed or added
PR title follows semantic commit guidelines
Search previous suggestions before making a new one, as yours may be a duplicate.
I acknowledge that all my contributions will be made under the project's license.

Notes:

Panquesito7

Thank you for your contribution and for your time in making this PR. 👍
Please add documentation and tests as seen in the typical structure of a program.

lazy-dude · 2021-07-10T09:37:11Z

I changed the code. Added tests and basic documentation. How should I send the file ?

lazy-dude · 2021-07-11T07:11:00Z

I pushed the commit.

Panquesito7

Please make a PR per change. Also, your code is still not up to the repository standards.
Take your time, have a seat, and read them carefully. 🙂

Panquesito7

Nice work! 😄👍

conversions/binary_to_decimal.c

Panquesito7 · 2021-07-16T19:56:20Z

conversions/binary_to_decimal.c

+ */
+bool is_binary(intmax_t num)
+{
+    int remainder = 0;


Consider using uint64_t for non-negative values (or their appropriate size: uint32_t, uint16_t, uint8_t) or int64_t for negative values. Requires adding the inttypes.h library. Check other parts of the code (reference). 🙂

uintmax_t has the biggest length for machine running the code. I would prefer it over uint64_t which has a fixed size.

Panquesito7 · 2021-07-16T19:57:21Z

conversions/binary_to_decimal.c

+// includes
+#include <assert.h>
+#include <stdbool.h>
+#include <stdint.h> 
 #include <stdio.h>
+#include <stdlib.h>


Please add a one-line description of what the library/header is for (see the example below).

#include <stdio.h> /// for IO operations #include <assert.h> /// for assert

@file Co-authored-by: David Leal <[email protected]>

@details Co-authored-by: David Leal <[email protected]>

@details Added ", the" Co-authored-by: David Leal <[email protected]>

Removed a line Co-authored-by: David Leal <[email protected]>

Removed: * Modified Co-authored-by: David Leal <[email protected]>

Suggested: main(void) -> main() Co-authored-by: David Leal <[email protected]>

static void test Co-authored-by: David Leal <[email protected]>

test arg doc Co-authored-by: David Leal <[email protected]>

Comment for test() Co-authored-by: David Leal <[email protected]>

Removed prototypes Co-authored-by: David Leal <[email protected]>

…tmax_t

Panquesito7

Please check the suggestions I made above if they're correct. If so, you can accept them. 🙂

conversions/binary_to_decimal.c

Panquesito7 · 2021-07-18T05:15:09Z

conversions/binary_to_decimal.c

+
+

Suggested change

conversions/binary_to_decimal.c

Panquesito7 · 2021-07-18T05:18:30Z

conversions/binary_to_decimal.c

+	unsigned remainder;
+	uintmax_t decimal_number = 0, temp = 1;
+
+    int length = num_len(UINTMAX_MAX) - 1;


Consider using uint64_t for non-negative values (or their appropriate size: uint32_t, uint16_t, uint8_t) or int64_t for negative values. Requires using the inttypes.h library. Check other parts of the code (reference). 🙂

@return

corrected @return Co-authored-by: David Leal <[email protected]>

Removed space Co-authored-by: David Leal <[email protected]>

Removed // includes Co-authored-by: David Leal <[email protected]>

@returns

two @returns Co-authored-by: David Leal <[email protected]>

@brief

Improved @brief Co-authored-by: David Leal <[email protected]>

Removed printf Co-authored-by: David Leal <[email protected]>

Added printf Co-authored-by: David Leal <[email protected]>

Panquesito7

Almost there! 😄

conversions/binary_to_decimal.c

Panquesito7 · 2021-07-21T20:02:46Z

conversions/binary_to_decimal.c

+	uint8_t remainder;
+	uintmax_t decimal_number = 0;
+	uint32_t temp = 1;
+
+    uint16_t length = num_len(UINTMAX_MAX) - 1;
+
+    assert(num_len(number) <= length);
+    assert(is_binary(number));


Wrong indentation. Please fix it.

Let's see if it works.

conversions/binary_to_decimal.c

@brief

@brief Change Co-authored-by: David Leal <[email protected]>

@param

@param added 'the ' Co-authored-by: David Leal <[email protected]>

@return

@return added 'a ' Co-authored-by: David Leal <[email protected]>

Panquesito7

Almost there! 😄

conversions/binary_to_decimal.c

Link removed Co-authored-by: David Leal <[email protected]>

@param

@param added 'the number' Co-authored-by: David Leal <[email protected]>

@brief

@brief link added Co-authored-by: David Leal <[email protected]>

conversions/binary_to_decimal.c

Removed a word Co-authored-by: David Leal <[email protected]>

it to that Co-authored-by: David Leal <[email protected]>

Added number Co-authored-by: David Leal <[email protected]>

printf indent Co-authored-by: David Leal <[email protected]>

Panquesito7

Almost there! 😄

conversions/binary_to_decimal.c

Panquesito7 · 2021-07-26T19:08:50Z

conversions/binary_to_decimal.c

+        remainder = num % 10;
+        if (remainder == 0 || remainder == 1) {
+            num /= 10;
+            continue;
+        } else
+            return false;
+    }
+    return true;


Please add a bit of documentation here of what this code does.

Added 'number' Co-authored-by: David Leal <[email protected]>

Added `` Co-authored-by: David Leal <[email protected]>

Panquesito7

Amazing work! Thank you for your dedication and contributions to our community! 😄🎉👍

lazy-dude added 4 commits July 8, 2021 15:26

Added asserts , number length check

67eeefe

Remove extra empty lines

0008c67

Auto indent

7185941

fix: corrected shown hex number.

6e5e561

Panquesito7 added the enhancement New feature or request label Jul 9, 2021

Panquesito7 requested changes Jul 9, 2021

View reviewed changes

Panquesito7 added Changes requested Proper Documentation Required requested to write the documentation properly labels Jul 9, 2021

Changed to recommended typical structure

905b80c

Panquesito7 requested changes Jul 14, 2021

View reviewed changes

docs: Added some doxygen doc.

e771021

Panquesito7 changed the title ~~Improved on conversion code , length variable , asserts added~~ [feat/fix/docs]: Improved on conversion code , length variable , asserts added Jul 16, 2021

Panquesito7 requested changes Jul 16, 2021

View reviewed changes

lazy-dude and others added 11 commits July 17, 2021 13:21

Update conversions/binary_to_decimal.c

157f6d1

@file Co-authored-by: David Leal <[email protected]>

Update conversions/binary_to_decimal.c

bbb6024

@details Co-authored-by: David Leal <[email protected]>

Update conversions/binary_to_decimal.c

82e1f41

@details Added ", the" Co-authored-by: David Leal <[email protected]>

Update conversions/binary_to_decimal.c

1e4c396

Removed a line Co-authored-by: David Leal <[email protected]>

Update conversions/binary_to_decimal.c

166dc0b

Removed: * Modified Co-authored-by: David Leal <[email protected]>

Update conversions/binary_to_decimal.c

57bcffa

Suggested: main(void) -> main() Co-authored-by: David Leal <[email protected]>

Update conversions/binary_to_decimal.c

129f4b0

static void test Co-authored-by: David Leal <[email protected]>

Update conversions/binary_to_decimal.c

8526600

test arg doc Co-authored-by: David Leal <[email protected]>

Update conversions/binary_to_decimal.c

61bed00

Comment for test() Co-authored-by: David Leal <[email protected]>

Update conversions/binary_to_decimal.c

4d91c9d

Removed prototypes Co-authored-by: David Leal <[email protected]>

Followed the two suggestions on code. Explain includes, intmax_t->uin…

8defb93

…tmax_t

Panquesito7 requested changes Jul 18, 2021

View reviewed changes

lazy-dude and others added 3 commits July 18, 2021 13:30

Update conversions/binary_to_decimal.c

6397733

corrected @return Co-authored-by: David Leal <[email protected]>

Update conversions/binary_to_decimal.c

45e4eff

Removed space Co-authored-by: David Leal <[email protected]>

Update conversions/binary_to_decimal.c

6553640

Removed // includes Co-authored-by: David Leal <[email protected]>

lazy-dude and others added 5 commits July 21, 2021 15:28

Update conversions/binary_to_decimal.c

fd82d9e

two @returns Co-authored-by: David Leal <[email protected]>

Update conversions/binary_to_decimal.c

ebf83bb

Improved @brief Co-authored-by: David Leal <[email protected]>

Update conversions/binary_to_decimal.c

63a9a16

Removed printf Co-authored-by: David Leal <[email protected]>

Update conversions/binary_to_decimal.c

a2ca660

Added printf Co-authored-by: David Leal <[email protected]>

Considered some suggestions

81ab47c

Panquesito7 requested changes Jul 21, 2021

View reviewed changes

Panquesito7 removed the Proper Documentation Required requested to write the documentation properly label Jul 21, 2021

lazy-dude and others added 4 commits July 22, 2021 14:39

Update conversions/binary_to_decimal.c

0f96de5

@brief Change Co-authored-by: David Leal <[email protected]>

Update conversions/binary_to_decimal.c

254cabf

@param added 'the ' Co-authored-by: David Leal <[email protected]>

Update conversions/binary_to_decimal.c

8c1b548

@return added 'a ' Co-authored-by: David Leal <[email protected]>

Added link for explaination.

8632490

Panquesito7 requested changes Jul 22, 2021

View reviewed changes

conversions/binary_to_decimal.c Outdated Show resolved Hide resolved

conversions/binary_to_decimal.c Outdated Show resolved Hide resolved

conversions/binary_to_decimal.c Outdated Show resolved Hide resolved

lazy-dude and others added 4 commits July 23, 2021 14:03

Update conversions/binary_to_decimal.c

8fa92c5

Link removed Co-authored-by: David Leal <[email protected]>

Update conversions/binary_to_decimal.c

4ab5a25

@param added 'the number' Co-authored-by: David Leal <[email protected]>

Update conversions/binary_to_decimal.c

da73878

@brief link added Co-authored-by: David Leal <[email protected]>

return -> returns

71c17ee

Panquesito7 requested changes Jul 26, 2021

View reviewed changes

conversions/binary_to_decimal.c Outdated Show resolved Hide resolved

conversions/binary_to_decimal.c Outdated Show resolved Hide resolved

conversions/binary_to_decimal.c Outdated Show resolved Hide resolved

conversions/binary_to_decimal.c Outdated Show resolved Hide resolved

lazy-dude and others added 4 commits July 26, 2021 11:31

Update conversions/binary_to_decimal.c

c3433f7

Removed a word Co-authored-by: David Leal <[email protected]>

Update conversions/binary_to_decimal.c

53e719b

it to that Co-authored-by: David Leal <[email protected]>

Update conversions/binary_to_decimal.c

af4b346

Added number Co-authored-by: David Leal <[email protected]>

Update conversions/binary_to_decimal.c

aa8e55b

printf indent Co-authored-by: David Leal <[email protected]>

Panquesito7 requested changes Jul 26, 2021

View reviewed changes

lazy-dude and others added 3 commits July 27, 2021 16:26

Update conversions/binary_to_decimal.c

0d61a4a

Added 'number' Co-authored-by: David Leal <[email protected]>

Update conversions/binary_to_decimal.c

82fb3ae

Added `` Co-authored-by: David Leal <[email protected]>

Added description

9612325

Panquesito7 approved these changes Jul 27, 2021

View reviewed changes

Panquesito7 added approved Approved; waiting for merge and removed Changes requested labels Jul 27, 2021

Panquesito7 requested review from ayaankhan98 and mishraabhinn July 27, 2021 18:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[feat/fix/docs]: Improved on conversion code , length variable , asserts added #840

[feat/fix/docs]: Improved on conversion code , length variable , asserts added #840

lazy-dude commented Jul 8, 2021 •

edited

Loading

Panquesito7 left a comment

lazy-dude commented Jul 10, 2021

lazy-dude commented Jul 11, 2021

Panquesito7 left a comment

Panquesito7 left a comment

Panquesito7 Jul 16, 2021

lazy-dude Jul 21, 2021

Panquesito7 Jul 16, 2021

Panquesito7 left a comment

Panquesito7 Jul 18, 2021

Panquesito7 Jul 18, 2021

Panquesito7 left a comment

Panquesito7 Jul 21, 2021

lazy-dude Jul 22, 2021

Panquesito7 left a comment

Panquesito7 left a comment

Panquesito7 Jul 26, 2021

Panquesito7 left a comment

[feat/fix/docs]: Improved on conversion code , length variable , asserts added #840

Are you sure you want to change the base?

[feat/fix/docs]: Improved on conversion code , length variable , asserts added #840

Conversation

lazy-dude commented Jul 8, 2021 • edited Loading

Description of Change

References

Checklist

Panquesito7 left a comment

Choose a reason for hiding this comment

lazy-dude commented Jul 10, 2021

lazy-dude commented Jul 11, 2021

Panquesito7 left a comment

Choose a reason for hiding this comment

Panquesito7 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Panquesito7 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Panquesito7 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Panquesito7 left a comment

Choose a reason for hiding this comment

Panquesito7 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Panquesito7 left a comment

Choose a reason for hiding this comment

lazy-dude commented Jul 8, 2021 •

edited

Loading