square root using Babylonian method returns wrong value?

Question

For the function isqroot() to calculate the square root using Babylonian method with one degree of precision and return it in a struct.

I'm unable to return the value to the struct and when I compile it is returning garbage value.

Here is my code:

#include<stdio.h>
#include<stdlib.h>
#include<math.h>

struct rootStruct {
    int rootInt;
    int rootFraction;
};

typedef struct rootStruct RootStruct;
RootStruct* isqroot (int n) {
    /*We are using n itself as initial approximation*/
    RootStruct* root=(RootStruct* )malloc(sizeof(RootStruct));
    float x = n;
    float y = 1;
    float e = 0.1; /* e decides the accuracy level*/

    while(x - y > e) {
        x = (x + y)/2;
        y = n/x;
    }
    root->rootInt = (int)x/2;
    root->rootFraction = (int)(x-root->rootInt)*100;
    return root;
}

int main(){
    RootStruct* roo+t=(RootStruct* )malloc(sizeof(RootStruct));
    printf("the sqrt is %d\n%d\n",root->rootInt,root->rootFraction);
    return 0;
}

What is wrong with this code?

Did it even compile ? And you didn't even call your function . — ameyCU, Sep 23 '15 at 09:54
In your design, you allocate a small amount of memory to hold the resulting root. If you allocate memory with `malloc`, you must also `free` it, which is tedious. Unlike arrays, structs can be passed around as values, so you could rewrite this code without pointers and without extra memory management overhead. — M Oehm, Sep 23 '15 at 11:49
@MOehm, passing a struct, that is greater than some very minimal size results in a number of hidden calls to memcpy() and memory being set aside for the memcpy() calls to use. That memory cannot be used for anything else. I.E. it is a very bad practice to pass structs by passing the whole struct. It is a much better practice to pass a pointer to the struct. — user3629249, Sep 26 '15 at 02:13
when calling the system function: `malloc()`, always check (!=NULL) the returned value to assure the operation was successful — user3629249, Sep 26 '15 at 02:16
@user3629249: The struct in question "weighs" two ints, which amounts to the size of a pointer in modern systems. I doubt that it is copied via `memcpy`. Also note that no struct is passed in, but a struct is returned. Returning a pointer here has further consequences: The pointer must point to static memory or to newly allocated memory. Static memory means that the result is overwritten by subsequent calls; allocated memory means that the calling code must explicitly free it. In my opinion returning a struct here is the better option if you want to kepp the `x = sqrt(8)` semantics. — M Oehm, Sep 26 '15 at 06:10

score 2 · Answer 1 · edited May 23 '17 at 12:06

You never call isqroot()... so root->rootInt and root->rootFraction are never set.

You also have a typo in

RootStruct* roo+t=(RootStruct* )malloc(sizeof(RootStruct));

it should be

RootStruct* root=(RootStruct* )malloc(sizeof(RootStruct));

without the +. However, this is unnecessary as you allocate memory in isqroot() and should probably be replaced by

RootStruct* root = isqroot(9);

Then don't forget to free() the memory at the end of main().

Just to note, you also shouldn't case the result of malloc() in C.

You have also implemented the algorithm incorrectly, it should be

RootStruct* isqroot (int n) {
    RootStruct* root=(RootStruct* )malloc(sizeof(RootStruct));

    float y = 1;
    float e = 0.1; /* e decides the accuracy level*/

    while(fabs(n - y*y) > e) {
        y = (y + (n / y)) / 2;
    }

    root->rootInt = (int) y;
    root->rootFraction = (int) (y - root->rootInt) * 100;

    return root;
}

where the complete corrected program is then

#include <stdio.h>
#include <stdlib.h>
#include <math.h>

struct rootStruct {
    int rootInt;
    int rootFraction;
};

typedef struct rootStruct RootStruct;

RootStruct* isqroot (int n) {
    RootStruct* root=(RootStruct* )malloc(sizeof(RootStruct));

    float y = 1;
    float e = 0.1; /* e decides the accuracy level*/

    while(fabs(n - y*y) > e) {
        y = (y + (n / y)) / 2;
    }

    root->rootInt = (int) y;
    root->rootFraction = (int) (y - root->rootInt) * 100;

    return root;
}

int main() {
    RootStruct* root = isqroot(9);
    printf("The sqrt is %d.%d\n", root->rootInt, root->rootFraction);
    return 0;
}

The Wikipedia article on Computing Square Roots has a very easy to understand section on the Babylonian method.

Umm ..I don't think it will work. I don't mean that you are wrong . But why declaraing `root` inside function and also in `main` ? — ameyCU, Sep 23 '15 at 10:03
@ameyCU Which part? The code in `isqroot()` seems broken to me but I've answered the question about garbage values due to `isqroot()` not being called. — IKavanagh, Sep 23 '15 at 10:04
Its his code that is not going to work even after the changes you told I think . Unnecessary casts and many things . — ameyCU, Sep 23 '15 at 10:09
@ameyCU Yes I agree, his code doesn't work even after the changes I've made. He has implemented the algorithm incorrectly. I've answered his problem with not being able to return the `struct` and getting garbage values. — IKavanagh, Sep 23 '15 at 10:14

square root using Babylonian method returns wrong value?

1 Answers1