Thread: help with function

  1. #1
    Registered User
    Join Date
    Feb 2015
    Posts
    1

    help with function

    Hello guys,
    I would like to ask you if anybody knows how to fix or upgrade my function "set". My teacher wrote me that I have to allocate the function inside not in main. And second thing is how to change that the pointers to pointers in function would be simplified.
    Thanks for help

    Code:
    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    
    #define BUFFER_SIZE 128
    
    int set(char *in, char **out);
    
    int main(int argc, char **argv)
    {
      char inStr[BUFFER_SIZE] = { 0 };
      char *outStr = NULL;
      int changed = 0;
      printf("Exercise 10.1\n");
    
      printf("Enter string:");
      if (!fgets(inStr, BUFFER_SIZE, stdin)) {  //enter string
    
        printf("Error load string...\n");
        exit(EXIT_FAILURE);
      }
      int lenght = strlen(inStr);   //include '\n' char as last
      inStr[lenght - 1] = '\0';     //remove last char '\n' from fgets function
      outStr = (char *) malloc(lenght); //alloc new string of lenght
      if (!outStr) {
        printf("Error in alloc memory...\n");
        exit(EXIT_FAILURE);
      }
      changed = set(inStr, &outStr);  //call function
      printf("Original string: %s\n", inStr);
      printf("Changed string: %s and changed letters: %d\n", outStr, changed);
      free(outStr);                 //free alloc memory
      getchar();
      return 0;
    }
    
    int set(char *in, char **out)
    {
      char *tmpString = *out;       //temp variable, out cannot be used like out++
      int changed = 0;              //changed letters
      while (*in) {
        if (*in >= 65 && *in <= 90) {
          *tmpString = *in + 32;
          changed++;
        } else if (*in >= 97 && *in <= 122) {
          *tmpString = *in - 32;
          changed++;
        } else {
          *tmpString = *in;
        }
        in++;
        tmpString++;
      }
      *tmpString = '\0';            //add ending char '\0'
      return changed;
    }
    Last edited by Salem; 02-09-2015 at 03:18 PM. Reason: fixed train-wreck colour/font

  2. #2
    Registered User
    Join Date
    Nov 2010
    Location
    Long Beach, CA
    Posts
    5,909
    First things first: next time, please post your code as plain text. That way the forum's software will be able to properly apply syntax highlighting and line numbering to your code.
    Issues I see:
    You don't use argc or argv, so you can declare main as int main(void) for simplicity and clarity.
    Code:
    int lenght = strlen(inStr); //include '\n' char as last
    inStr[lenght - 1] = '\0'; //remove last char '\n' from fgets function
    1. It's spelled length. Minor issue, but liable to cause annoying problems if you spell it correctly later .
    2. This does not necessarily remove the '\n' from the end of the string. It removes the last character no matter what it is. Most of the time it will be a '\n', but it may be something else (e.g. the last letter of the user's input because the input was terminated with EOF instead of '\n')
    Instead, try using the strchr method found here: FAQ > Get a line of text from the user/keyboard (C) - Cprogramming.com.
    Code:
    if(!fgets(inStr, BUFFER_SIZE, stdin)) { //enter string
    It is preferable to use sizeof(inStr) in the fgets call, instead of BUFFER_SIZE. That way, if you ever declare inStr to be a different size (e.g. FOO_SIZE), your fgets call will still be correct, you wont need to change it. Not a serious issue in a smaller program, but a good habit to get into so it's automatic when you write larger, more complex programs.
    [/code]outStr = (char*)malloc(lenght); //alloc new string of lenght[/code]
    Don't cast the return value of malloc, see here: FAQ > Casting malloc - Cprogramming.com. Also, since there might not be a newline, you need to make sure you always allocate enough space for the whole new string, plus the

    set() is a horrible funciton name. I have no idea what it does. What is it setting? Looks more like it's swapping certain characters. How about you call it swapXandY, where you pick the appropriate words to fill in X and Y so the function name is accurate and descriptive. Never sacrifice clarity of code to save a few keystrokes. It saves you 30 seconds now, and can cost you hours later, if you have to return to your code. Again, not a serious issue in a smaller program, but form good habits early on.

    Don't use magic numbers. I can guess what 65, 90, 97, 122 and 32 are for, but it's really not clear. Also, while ASCII or compatible character sets are very, very common, they are not universal. There do still exist character sets (like EBCDIC) where upper case letters are not between 65 and 90, and they are not consecutive, and they are not 32 chars away from their lowercase counterparts. Instead, check for upper/lower case with the isupper() and islower() functions, and convert them with toupper() and tolower(). You will need to #include <ctype.h> to use those function. This gives you a few benefits:
    1. It is more portable: it will work no matter what character set you end up using.
    2. You don't have to worry about remembering what numbers are what.
    3. It uses pre-built and pre-tested functions, so you don't need to worry about making a mistake, the logic is already handled for you -- no need to reinvent the wheel.
    4. It makes your code more readable. Those function names describe clearly what they do, so your code will be much easier to read:
    Code:
    if (isupper(*in)) {
        *tmpString = tolower(*in);
    } else if (islower(*in)) {
        *tmpString = toupper(*in);
    } else {
        *tmpString = *in;
    }
    To malloc inside the function, and make sure outStr is correct in main, simply assign the result of malloc to *out in your swapXandY function
    Code:
    *out = malloc(strlen(in) + 1);
    Make sure you check the return value of malloc, to ensure you actually have memory before you attempt to write to it.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Function Prototype, Function Call, and Function definition
    By dmcarpenter in forum C Programming
    Replies: 9
    Last Post: 04-09-2013, 03:29 AM
  2. Replies: 13
    Last Post: 03-20-2012, 08:29 AM
  3. Replies: 15
    Last Post: 06-09-2009, 02:19 AM
  4. Print function: sending a function.. through a function?
    By scarlet00014 in forum C Programming
    Replies: 3
    Last Post: 11-05-2008, 05:03 PM
  5. Replies: 9
    Last Post: 01-02-2007, 04:22 PM

Tags for this Thread