Thread: Doing 2 things at the same time

  1. #16
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    OK, you win
    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.

  2. #17
    Registered User
    Join Date
    Jan 2008
    Posts
    182
    Ok, well I'll not make it volatile.

    I did something, wrote a program that generates semiprime numbers, from 1 to 10 million, and while doing that it shows the moving sticks thingy. When It finishes, it prints everything to a file, showing progress without using a second thread.

    I'll post the code after I clean it up, thing is that it is a big mess right now 'cause I got code from 3 other programs I had.

    Thanks a lot.

  3. #18
    Registered User
    Join Date
    Jan 2008
    Posts
    182
    I attached the code.

    It works nicely, that's what I wanted. I am thinking now of opening another thread that would then tell me the time it took to generate the semiprimes, but I'm not sure of how to count time by the millisecond.

    If you guys see any bad coding practices then please tell me. I have less than a year of experience so most of the time I don't know if I do things like they should be done.

    Thanks.

  4. #19
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    I would think a thread is excess for this task.
    Just make a call to a timing function before and after, then compare the times.
    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.

  5. #20
    Registered User C_ntua's Avatar
    Join Date
    Jun 2008
    Posts
    1,853
    Saw your code kind of quick. I think it is fine!
    I don't think you have any bad code practices. Well, except that you change your "styling rules". Like leaving a space after = sometimes, others not. But this isn't bad except if you plan to have your code written from other co-programmers. Or if you seek perfection :P

  6. #21
    Technical Lead QuantumPete's Avatar
    Join Date
    Aug 2007
    Location
    London, UK
    Posts
    894
    In your progress_indicator, rather than while(1), I'd do while (!gb_exit). I'm pretty sure the thread will terminate once it reaches the end of the function. Also you're not initialising your pointers to NULL.

    QuantumPete
    "No-one else has reported this problem, you're either crazy or a liar" - Dogbert Technical Support
    "Have you tried turning it off and on again?" - The IT Crowd

  7. #22
    Registered User
    Join Date
    Jan 2008
    Posts
    182
    Elysia:
    Yes, I did some research and found out that I can do something like:
    Code:
    ticks = clock();
    at the beginning of the prime generating function and after that, something like
    Code:
    ticks = clock() - ticks;
    printf("%.1f", ticks / (double)CLOCKS_PER_SEC);
    C_ntua:
    Yes, I said before that it was a mess because I sometimes change of style, like the K&R way instead (that way I can get used to different coding styles). Since that program is made up of 3 different programs I had written before, they were all different But lately I've been writing in K&R style, leaving spaces between operators if they are small expressions.

    QuantumPete:
    Thanks a lot, looks like good idea doing both things.

    Thanks.

  8. #23
    Registered User Codeplug's Avatar
    Join Date
    Mar 2003
    Posts
    4,981
    code review:

    >> #include <stdbool.h>
    Suggestion: C99 only header. To make this explicit add:
    Code:
    #if __STDC_VERSION__ < 199901L
    #   error "C99 Source"
    #endif
    >> #ifdef WIN32
    Your code is currently windows specific, so this isn't really needed. If you're really interested in running this code on multiple platforms, let us know. Otherwise remove anything that implies the code is multi-platform.

    >> #define VAL_FORMAT "%I64u" (vs. "%llu")
    The latest MS-CRT supports "%ll". For MS compilers, the best way (that I know of) to detect the major MS-CRT version is to check the compiler's version number. In this case the CRT that comes with VS-2005 and up supports "long long" and "%ll".
    For MinGW (which uses the MS-CRT), they were nice enough to include a __MSVCRT_VERSION__ macro (from headers, not from compiler). They don't support the latest CRT, but perhaps someday. So a "better" conditional compile would be to detect when the non-standard I64 should be used, then default to the standard:
    Code:
    #if (defined(_MSC_VER) && (_MSC_VER < 1400)) || \
        (defined(__MSVCRT_VERSION__) && (__MSVCRT_VERSION__ < 0x0800))
    #   define I64_FORMATA "%I64"
    #else
    #   define I64_FORMATA "%ll"
    #endif
    
    #define VAL_FORMAT I64_FORMATA "u"
    I can't speak for the free Borland compiler and runtime. I also don't know if it was your intention to support multiple Windows compilers since you merged from multiple sources.

    >> typedef unsigned long long u_long_64;
    You could use the same conditional compile above, or just use __int64. Not all windows compilers support long long. Most do (or can) support __int64. (MinGW defines __int64 as long long for instance).

    >> bool gb_exit = 0;
    Unsynchronized access (read & write) is "naughty" and should never be done (even if it "works" as it's being used here). A better way is to use a Win32 event:
    Code:
    HANDLE gev_exit = 0;
    
    //...in main, create the event
    gev_exit = CreateEvent(NULL, TRUE, FALSE, NULL);
    if (!gev_exit)
    {
        printf("CreateEvent failed, le = %d", GetLastError());
        return 1;
    }
    
    //...in the thread, wait for the event and remove call to Sleep()
    while(WaitForSingleObject(gev_exit, 1000) != WAIT_OBJECT_0) 
    {
        printf("\b");
    ...
    
    //...when you want the thread to exit
    SetEvent(gev_exit);
    Don't stop the thread from within semiprimegen(), it shouldn't have to know anything about it. Stop the thread within main() where it was created.

    >> bool line_number_switch = 0
    Suggestion: Use true and false with all bool types for readability and consistency.

    >> _beginthread(progress_indicator, 0, NULL);
    Check return, handle errors.

    >> fpurge()
    This is old, nonstandard, and doesn't do what fflush() does. I can't see any reason why you would use it on any platform.

    >> primefile = fopen()
    None of the file function calls are checking for errors.

    >> if(line_number_switch==0)
    Since this is done only once, move it outside the for loop and do it once unconditionally.

    >> if(filecount==10)
    Suggestion: A better variable name would be rowcount.

    >> 100.0 * x / semiprimes_count
    Suggestion: Using explicit parenthesis is generally considered more readable and safer. Being explicit about your intended order of operations (with parens) will prevent bugs in those cases where parens are really required and you forget to add them.

    >> (int)(100.0 * x / semiprimes_count)
    Drop the ".0", floating point math isn't needed since integer division will give the same results in this case.

    >> #define BUFFER 1000000
    "Magic number". Move this inside semiprimegen() as a constant.
    const u_long_64 buff_sz = 1000000;

    >> if(store_2)
    Again, done only once, move outside the for loop. Or better, initialize buff_mult to 0 so that the initial allocation of primes_address is performed the same as for semi_prime_address in the next for loop.

    >> (u_long_64*)malloc()
    It's considered good practice not to cast the result of malloc/realloc in C.
    http://c-faq.com/malloc/cast.html

    >> *(primes_address + prime_count-1) = number_to_check;
    Suggestion: Array subscript notation may be a little more readable. More so in other parts of the code.
    primes_address[prime_count-1] = number_to_check;

    Overall:
    Everywhere I've worked, the coding standards didn't allow tabs in the source, and they didn't allow code or comments past the 80'th column. Something to consider.

    There are 4 warnings due to conversion from u_long_64 to size_t. I would consider changing some "counters" to type size_t if you know they will never exceed 64 bits. If they ever do exceed 64 bits, then you'll have problems. Ensure your compilation options have all warning turned on. Get into the habbit of treating warnings as errors.

    "x" is an unreferenced local in semiprimegen().

    gg

  9. #24
    Registered User
    Join Date
    Jan 2008
    Posts
    182
    wow, you checked absolutely everything, thanks!

    I'll start working on that one by one now, I'll ask questions if I don't understand something.

    First question:
    Everywhere I've worked, the coding standards didn't allow tabs in the source, and they didn't allow code or comments past the 80'th column. Something to consider.
    What do you mean didn't allow tabs? As in I shouldn't indent my code using Tab??? I do understand the 80th column and I bet the reasons are readability without having to side scroll, especially if you are reading the code from terminal/shell/console, but how come not use tab? What am I supposed to use then?

    Thanks.

  10. #25
    Registered User Codeplug's Avatar
    Join Date
    Mar 2003
    Posts
    4,981
    >> What do you mean didn't allow tabs?
    Tab "characters", which is "char(0x09)".

    >> What am I supposed to use then?
    Spaces. Many editors have an option to "convert tabs to spaces" - so you can still physically hit your tab key on the keyboard.

    The reasoning behind it is that editors are free to treat tab characters as any number of spaces. A tab may be 3 spaces in your editor, and 9 spaces in another.

    80 column rule is also meant to ensure the printed code looks the same as it does on the screen. In practice, not having to side-scroll is the more usefully reason (for me).

    Both rules help ensure consistent viewing of the code, on paper and within all editors.

    For those interested, I looked up how Borland treats 64bit ints. They use __int64 and "L" as the size prefix. So here's a more complete conditional compile for I64_FORMATA:
    Code:
    #if (defined(_MSC_VER) && (_MSC_VER < 1400)) || \
        (defined(__MSVCRT_VERSION__) && (__MSVCRT_VERSION__ < 0x0800))
    #   define I64_FORMATA "&#37;I64"
    #elif defined(__BORLANDC__)
    #   define I64_FORMATA "%L"
    #else
    #   define I64_FORMATA "%ll"
    #endif
    gg

  11. #26
    Registered User
    Join Date
    Jan 2008
    Posts
    182
    OK, I changed some stuff

    - line_number_switch removed
    - store_2 removed
    - expressions involving bools changed to false or true instead of 0 or 1
    - removed x, file_linecount and perc from semiprimegen function (no idea why I had them there)
    - changed name from filecount to rowcount on main function

    - deleted every #ifdef WIN32
    - deleted fpurges too
    - changed the unsigned long long format conditional statements to your suggestion
    - stdbool gives me errors if I check for it in your suggested way, so I left it as is

    -added error checking for _beginthread
    -added error checking for fopen
    -added fclose, I forgot to close the file at the end of main
    -added error checking for fclose

    -removed gb_exit variable
    - removed cast to (u_long_64 *) from mallocs and reallocs
    - changed parts of code to array notation for better readability

    -weird perc = (int)(100 * x / semiprimes_count)... if I put
    Code:
    (int)(100 * (x / semiprimes_count))
    it doesn't work, no Idea why, even if I cast it.
    Code:
    (int)(100 * (double)(x / semiprimes_count))
    but,
    Code:
    (int)((100 * x) / semiprimes_count)
    does work. Any Idea why that happens???

    - changed the way progress_indicator checks when semiprimegen finished,
    ... this is my function now
    Code:
    void progress_indicator(void *dummy) 
    {
        u_short steps = 0;
        
        while(1) 
        {
            // print the first character "|"
            switch(steps) 
            {
                case 0:
                    printf("|");
                    break;
                case 1:
                    printf("/");
                    break;
                case 2:
                    printf("-");
                    break;
                case 3:
                    printf("\\");
                    break;
                default:
                    break;
            }
            if(steps < 3)
                steps++;
            else
                steps = 0;
            
            if(WaitForSingleObject(gev_exit, 1000) == WAIT_OBJECT_0)
                _endthread();
            else 
                printf("\b");
        }
    }
    Is that way ok, Or do I modify the while(1)? If I do that, I'll have to print the first "|" out of the loop...... or maybe I could use a do while in this case...

    Oh and, why does the code you gave me to make stdbool.h C99 explicit gives me an error in my compiler? Is it because I don't have my compiler set to C99 or what? If I take the code away, it works like it should anyways.

    Also, My compiler doesn't give me errors about changing from u_long_64 to size_t. I'm using MinGW.

    I'm also using Notepad++, I'll check if the option of convert tabs to spaces is there. I can see that if I post code here, firefox separates tabs with 8 spaces, while Notepad++ is 4.

    Oh and yes, I wanted to make this portable to Linux, but apparently process.h and all those thread functions are Windows specific.

    Thanks.
    Last edited by samus250; 06-19-2008 at 02:34 PM. Reason: better formatting

  12. #27
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    Oh yes, WaitForSingleObject and CreateEvent are Windows-specific.
    About the problem with the division: I'd suggest you keep the original. There is a difference. Your original code computes the equation in floating point, then truncates it. But the new version doesn't calculate using decimals at all, which may give misleading result (especially with procent calculation).
    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.

  13. #28
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    I'm assuming x and semiprimes_count are ints. Then the division will be done integer-style (no remainder) -- you have to cast x or semiprimes_count to double before the division happens (casting the result of the division is Too Late).

  14. #29
    Registered User
    Join Date
    Jan 2008
    Posts
    182
    Ok let me get this straight

    An expression like that one
    100 * x / semiprimes_count

    Will first calculate everything form left to right, truncating the floating points as it calculates.
    Now, if x and semiprimes_count are both double and I use 100.0, it won't truncate.

    But, 100 * (x / semiprimes_count) is calculated like two expressions
    first, x / semiprimes_count
    and then 100 * that.

    But since x and semiprimes_count are ints, the result of x / semiprimes_count will be truncated to 0 (since i'm calculating percentage, semiprimes_count will always be greater than or equal to x) BEFORE multiplying times 100, and if I cast it to double, the result will be 0.0.
    So
    Code:
    (x / semiprimes_count); // will equal 0
    (double)(x / semiprimes_count); // will equal 0.0
    ((double)x / (double)semiprimes_count); // will equal 0.73 (73 percent, for example)
    Is that it?
    Last edited by samus250; 06-19-2008 at 02:31 PM. Reason: wrote "to" instead of "two" hehe

  15. #30
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    We have two terms: integer division and floating division.
    If one side of the expression is a floating, we get floating division. If both sides are integers, we get integer division.
    How exactly it works is platform, compiler and CPU dependant, though, so we can't say for sure if it's just truncated or not.
    (We'd have to pick a specific platform, compiler and processor to say that for certain, because the C standard itself only says what the outcome is, not how it's performed.)

    Either way, in floating division, the result is a floating point number. In integer division, it's an integer that's the result.
    Therefore, to calculate things that require floating (like &#37, you need to make sure one side of the expression is a floating.
    You can't cast the result because the result itself isn't a floating, and you can't restore lost data.

    We can actually make the compiler treat a number differently in two ways. One by appending a prefix to the number to tell the compiler what type it is or by casting the number to the correct type.
    For the first, we append a "." to the number. So if 100 is an integer, then 100.0 would be a double. If we add "f", it would become a float: 100.0f = float.
    We can also do (double)100 or (float)100, which works just fine as well, but the former is more common.

    10 / 100 = 0
    10.0 / 100 = 0.1
    10 / 100.0 = 0.1
    Last edited by Elysia; 06-19-2008 at 02:36 PM.
    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. Using pointers
    By Big_0_72 in forum C Programming
    Replies: 3
    Last Post: 10-28-2008, 07:51 PM
  2. Determine the closest departure time
    By Kyeong in forum C Programming
    Replies: 9
    Last Post: 10-07-2008, 08:06 PM
  3. Execution Time - Rijandael encryption
    By gamer4life687 in forum C++ Programming
    Replies: 5
    Last Post: 09-20-2008, 09:25 PM
  4. Pthreads performance
    By C_ntua in forum C Programming
    Replies: 42
    Last Post: 06-17-2008, 11:29 AM
  5. time class
    By Unregistered in forum C++ Programming
    Replies: 1
    Last Post: 12-11-2001, 10:12 PM