Thread: qsot array of Time objects - help

  1. #1
    Registered User
    Join Date
    Mar 2012
    Posts
    4

    qsot array of Time objects - help

    I have been trying to figure this out for the past few hours so it's about time i search for help.

    I was to use the Time class library and sort an array of time objects using the qsort function. I beleive my compare function is the problem and have tried different variations but still can't seem to grasp it.

    Code:
    #include <iostream>
    #include "ccc_time.h"
    
    
    using namespace std;
    int tCompare(const void* , const void*) ;
    
    void main()
    {
        
    
        Time* times[7] = {new Time(10, 13, 55), new Time(15, 13, 59), new Time(18, 7, 20), new Time(16, 10, 39),
                          new Time(9, 13, 45), new Time (16, 15, 24), new Time(17, 5, 8)};
        
    
        
    
        cout << "unsorted times: " << endl;
    
        for (int i =0; i < 7; i++)
            cout<< times[i]->get_hours() << ":" << times[i]->get_minutes() << ":" << times[i]->get_seconds() << endl;
        
        /* can't get to sort, beleive its something wrong with my compare function */
        qsort(times, 7, sizeof(Time), tCompare);
        
        cout << endl;
        cout << "sorted times: " << endl;
    
        for (int i =0; i < 7; i++)
            cout<< times[i]->get_hours() << ":" << times[i]->get_minutes() << ":" << times[i]->get_seconds() << endl;
        
        system ("pause");
    }
    
    int tCompare(const void* ptr1, const void* ptr2) 
    
    {
        Time time1, time2;
    
        time1 = *(Time *)ptr1;  
        time2 = *(Time *)ptr2;  
        
        /* can't get to sort, beleive its something wrong with my compare function,
        this is not frist variation iv'e tried of the compare*/
        
        if(time2.get_hours() - time1.get_hours() > 0)
            return -1;
        else if (time2.get_hours() - time1.get_hours() == 0 && time2.get_minutes() - time1.get_minutes() > 0)
            return -1;
        else if (time2.get_hours() - time1.get_hours() == 0 && time2.get_minutes() - time1.get_minutes() == 0
                && time2.get_seconds() - time1.get_seconds() > 0)
            return -1;
        else if (time2.get_hours() - time1.get_hours() == 0 && time2.get_minutes() - time1.get_minutes() == 0
                && time2.get_seconds() - time1.get_seconds() == 0)
            return 0;
        else
            return 1;
    }

  2. #2
    Registered User
    Join Date
    Mar 2012
    Posts
    4
    I've got it to work. I made my array a normal array instead of an array of pointers. I don't understand how that fixed it but it works.

  3. #3
    Registered User
    Join Date
    Oct 2006
    Posts
    3,445
    my first question is why are you accepting void* parameters in the function, and then casting, using the incorrect, c-style cast, to Time*? that makes absolutely no sense in a strongly-typed language like C++.

  4. #4
    - - - - - - - - oogabooga's Avatar
    Join Date
    Jan 2008
    Posts
    2,808
    Your first incarnation didn't work because of these lines:
    Code:
        Time time1, time2;
        time1 = *(Time *)ptr1; 
        time2 = *(Time *)ptr2;
    It should be
    Code:
        Time *time1, *time2;
        time1 = (Time *)ptr1; 
        time2 = (Time *)ptr2;
    and you access them like
    Code:
    time2->get_hours() - time1->get_hours()
    etc.
    You could rewrite it something like this:
    Code:
    int tCompare(const void* ptr1, const void* ptr2) 
    {
        Time *time1 = (Time *)ptr1;
        Time *time2 = (Time *)ptr2;
    
        // change int to whatever type hours, minutes, and seconds are
        int dh = time2->get_hours()   - time1->get_hours();
        int dm = time2->get_minutes() - time1->get_minutes();
        int ds = time2->get_seconds() - time1->get_seconds();
    
        if (dh > 0 || dh == 0 && dm > 0 || dh == 0 && dm == 0 && ds > 0)
            return -1;
        return !(dh == 0 && dm == 0 && ds == 0);
    }
    The cost of software maintenance increases with the square of the programmer's creativity. - Robert D. Bliss

  5. #5
    Registered User
    Join Date
    Mar 2012
    Posts
    4
    Quote Originally Posted by oogabooga View Post
    Your first incarnation didn't work because of these lines:
    Code:
        Time time1, time2;
        time1 = *(Time *)ptr1; 
        time2 = *(Time *)ptr2;
    It should be
    Code:
        Time *time1, *time2;
        time1 = (Time *)ptr1; 
        time2 = (Time *)ptr2;
    and you access them like
    Code:
    time2->get_hours() - time1->get_hours()
    etc.
    Thank you! This would explain why -> operators where not working in one of my versions that I tried. Also you illustrated what was wrong simply and clearly in a time efficient manner. Thank you very much.

  6. #6
    Registered User
    Join Date
    Oct 2006
    Posts
    3,445
    it would be better to define the function as follows:
    Code:
    int tCompare(const Time* time1, const Time* time2) 
    {
        // change int to whatever type hours, minutes, and seconds are
        int dh = time2->get_hours()   - time1->get_hours();
        int dm = time2->get_minutes() - time1->get_minutes();
        int ds = time2->get_seconds() - time1->get_seconds();
     
        if (dh > 0 || dh == 0 && dm > 0 || dh == 0 && dm == 0 && ds > 0)
            return -1;
        return !(dh == 0 && dm == 0 && ds == 0);
    }

  7. #7
    Registered User
    Join Date
    Mar 2012
    Posts
    4
    Thank you, cleaning up my code and making it more efficient is something I strongly need to work on.

  8. #8
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    It would be better to get rid of the pointers altogether. They are leaking memory because you aren't deleting.
    I still don't understand why no one has yet complained about void main. Change it to its proper int main, post haste.
    Last, but not least, a different way of doing this is to use operator overloading. You should try it just to get the hang of both ways.
    (Does not work with pointers, mind you!)

    Also have a look at:
    SourceForge.net: Do not remove parameter names - cpwiki
    SourceForge.net: Pause console - cpwiki
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. 2D array of objects
    By gavra in forum C++ Programming
    Replies: 20
    Last Post: 06-07-2009, 12:09 PM
  2. array of objects
    By adam_no6 in forum C++ Programming
    Replies: 2
    Last Post: 04-04-2007, 07:59 AM
  3. array of objects
    By Ideswa in forum C++ Programming
    Replies: 16
    Last Post: 04-23-2006, 08:59 AM
  4. array of objects
    By sbook in forum C++ Programming
    Replies: 6
    Last Post: 06-01-2005, 02:13 PM
  5. Replies: 4
    Last Post: 10-16-2003, 11:26 AM