tags:

views:

257

answers:

5

I'm working on a homework assignment in which I'm required to use char arrays instead of strings and qsort/bsearch. In my call to bsearch below, I know I'm passing the wrong size of Entry, but I'm not sure how to get the real size, and my compareEntries function is therefore not finding the right objects.

Can anyone help me understand what I'm missing?

#include  <iostream>

using  namespace  std;


typedef Entry*  EntryPtr;
class  Dictionary
{
    public  :
        Dictionary( const char  *filename );
        ~Dictionary();
        const char  *lookupDefinition( const char  *word );

    private  :
        int m_numEntries;
        EntryPtr *m_entries;
        static int compareEntries(const void *leftSide, const void *rightSide);
};

int Dictionary::compareEntries(const void *leftSide, const void *rightSide)
{
    EntryPtr lside = (EntryPtr) leftSide;
    EntryPtr rside = (EntryPtr) rightSide;

    return strcmp(lside->Word, rside->Word);
}

const char *Dictionary::lookupDefinition(const char *word)
{
    if (m_numEntries == 0)
        return 0;

    EntryPtr result = (EntryPtr) bsearch(word, m_entries, 
        m_numEntries, sizeof(m_entries[0]), Dictionary::compareEntries);

    return result->Definition;
}

class Entry
{
        public:
                Entry(const char *line);
                char *Word;
                char *Definition;
};

int  main()
{
    Dictionary  dict( "C:\\path\\file.txt" );
    dict.lookupDefinition("chair");
    return  0;
}
+2  A: 

Why doesn't sizeof(Entry) work?

Changed again -- I think the size should be the size of the pointer....

EntryPtr tmp = new Entry("");
tmp->Word = word;

EntryPtr result = (EntryPtr) bsearch(tmp, m_entries, 
        m_numEntries, sizeof(EntryPtr), Dictionary::compareEntries);
Hogan
Because Entry is a pointer.
JMP
Entry is not a pointer. EntryPtr is a typedef of a pointer type. This means you can use `sizeof(Entry)`.
Amit Kumar
@JMP : Did you try what I suggested in my answer?
Hogan
@Amit, he is using `sizeof(EntryPtr)` so it is still a pointer (since `m_entries` is an `Entry**`) I don't think this sizeof is his real problem though, see Mark Ransom's answer.
Adam W
m_entries is an array of pointers to Entry (a typedeffed pointer makes one's head spin, doesn't it?). The size of the item should therefore be the size of the pointer.
UncleBens
@Adam : His sizeof is wrong. It is the size of a pointer not an Entry. An entry is at least 3 pointers in size. See my solution posted above, uses both our points.
Hogan
@UncleBens : Sure arrays are strange... but this doc here (2nd example) suggests he needs the size of what is pointed to... http://www.cplusplus.com/reference/clibrary/cstdlib/bsearch/
Hogan
@UncleBens : NM I think you are probably right.
Hogan
@Hogan: Exactly, my comment says its the size of a pointer not an entry. `EntryPtr` is typecast to `Entry*` read carefully :)
Adam W
+1  A: 

You do know that bsearch requires sorted input, right?

sizeof(m_entries[0]) looks perfectly fine to me.

Edit: Now I see the problem. Your Dictionary class contains an array of pointers. The problem is in the compareEntries function, where you cast passed pointers to EntryPtr; you need to cast them to EntryPtr * instead.

Edit 2: As pointed out by Amit Kumar, you also need to change the key parameter you send to bsearch, or you need to realize that the pointers you receive in compareEntries are not pointing to the same types and will need two different typecasts.

Mark Ransom
I did not reveal the code where my input is sorted via qsort.
JMP
+1  A: 

The problem is that the comparator function used in bsearch expects word to be of type Entry* (or m_entries to be of type char**).

Amit Kumar
A: 

Read the manual carefully.

A summary of the points made by others, as well as a couple more issues:

  • Your usage of sizeof is correct.

  • You should pass a pointer to the Entry containing the key you want to look up. Actually the key can be anything, and it will be passed to the comparison function as the first argument, and you just need to cast both arguments to right types. (The comparison function should still correspond to the order the items were sorted by.)

  • The casts in the comparison function are not correct. The comparison function receives a pointer to the element (which in your case is a pointer to Entry, hence the comparison function receives pointers to pointers to Entry).

  • You cast the result to the wrong type. Again the function returns a pointer to an element in the array (pointer to pointer to Entry).

  • You don't check if the result is NULL, should the key not be there.

  • You could probably give up one level of indirection (do you really need an array of pointers instead of an array of Entries?)

  • You should take it as a good example of what people mean when they talk about the virtues of type-safety: in your code virtually all the types are mixed up and you are doing wrong things with wrong types, yet not a single complaint from the compiler. That's what you get if you mess with void*, unless you know exactly what you are doing.

For the fun of it, having an array of pointers it takes a ridiculous amount of indirection to get a result:

#include <cstdlib>
#include <string>
#include <iostream>

int compare_string(const void* a, const void* b)
{
    return ((const std::string*)a)->compare(**(const std::string**)b);
}

int main()
{
    std::string a("a"), b("b"), c("c");
    std::string* array[3] = { &a, &b, &c };
    std::string key = "b";
    std::string** result = (std::string**)bsearch(&key, array, 3, sizeof(std::string*), compare_string);
    if (result) std::cout << **result << '\n';
}

IMO, it would take less time to implement your own type-safe bsearch, than it takes to figure all this out and have it tested and debugged.

UncleBens
A: 

sizeof(Entry) would work. Mostly sizeof should be used on the type instead of an instance.

sizeof(Entry)

is preferable to

Entry e;
sizeof(e);

or

Entry* e;
sizeof(*e);

all give the same result.

#include "stdio.h"
class Entry {
  double e;
  int i;
};
int main() {
  Entry e;
  printf("%d\n", sizeof(e));
  printf("%d\n", sizeof(Entry));
  printf("%d\n", sizeof(*(&e)));
  return 0;
}
JohnH
It is preferable to apply sizeof to an instance, not a type name: if the type of the variable you are using is changed, `sizeof(var)` will still be correct, but `sizeof(TheTypeItUsedToBe)` necessarily won't.
UncleBens
Good point UncleBens. The original mistake was taking sizeof pointer (array item) and was a misunderstanding of pointer syntax, sizeof type would prevent, sizeof instance is probably better for the reasons UncleBens states.
JohnH
sizeof(Entry) is wrong, because the array is an array of pointers, not an array of Entrys.
Mark Ransom