#225 Unit test for bn_poly_synthetic_division() routine

Untested
closed-accepted
Sean Morrison
None
5
2013-08-16
2013-07-31
Nyah Check
No

this is the unit test for the bn_poly_synthetic_division routine.

1 Attachments

Discussion

  • Nyah Check
    Nyah Check
    2013-08-02

    modified with

    The known values used for these tests are generated from Octave
    GNU Octave, version 3.4.3
    Copyright (C) 2011 John W. Eaton and others.
    Octave was configured for "i386-redhat-linux-gnu".

     
  • Nyah Check
    Nyah Check
    2013-08-03

    modified with

    The known values used for these tests are generated from Octave
    GNU Octave, version 3.4.3
    Copyright (C) 2011 John W. Eaton and others.
    Octave was configured for "i386-redhat-linux-gnu".

    Debugged from errors resulting from wrong base values printing correct results

     
    Attachments
  • Nyah Check
    Nyah Check
    2013-08-06

    this patch modifies the indentation problems and should be applied after patch 224

     
    Attachments
  • Nyah Check
    Nyah Check
    2013-08-12

    modified patch with indentation problems fixed.

     
    Attachments
  • Cliff Yapp
    Cliff Yapp
    2013-08-12

    Nyah,

    I'm getting a segfault when I try to run this test. Is this expected?

     
  • Nyah Check
    Nyah Check
    2013-08-13

    its not supposed to give any seg faults. and did you use this patch? https://sourceforge.net/p/brlcad/patches/223/#ecb1 . But i'll recheck the patches.

     
    Last edit: Nyah Check 2013-08-13
  • Nyah Check
    Nyah Check
    2013-08-16

    corrected patch with tests working perfectly. :)

     
    Attachments
  • Sean Morrison
    Sean Morrison
    2013-08-16

    Looking good, but there are a few minor issues to keep in mind with any future work:

    1) Name mismatch. The file header doesn't match the file name and neither match the target. Should be the same or at least more consistent than this.

    2) Copyright year usually begins with the file, so it should have been 2013, not 2004-2013.

    3) Space before commas. You have whitespace errors on lines 78, 97, possibly others.

    4) You have an unreachable code error on line 121 in main() as the preceding if/else statement returns for both cases. The common pattern for that is:

    if (expression) {
    ...
    return ret;
    }

    return EXIT_FAILURE;

    5) If I change a remainder coefficient, at least rem[0].cf[3], the test still passes.

    6) Comment over test_bn_poly_syn_div() is wrong. Comment block over poly_init() is wrong. Comment block over poly_init() lacks spaces after the '' and should not be a doxygen comment (/ /).

    7) Introducing globals like bn_Zero_poly are discouraged. Fixing the public header to provide a static initializer should have been the course of action.

    8) Shouldn't be using globals or they should be marked with static scope.

    Any one of these issues in isolation would not be a problem. All of them together, however, indicate more work is still needed on your part to notice these issues.

    Applied in r56896 with numerous patch fixes following. Perhaps you can create a patch that fixes the other issues in the new bn unit tests that starseeker applied for you. They all have nearly the same issues as identified above.

     
  • Sean Morrison
    Sean Morrison
    2013-08-16

    • status: open --> closed-accepted
    • assigned_to: Sean Morrison