Make function more simple

Question

0.00/5 (No votes)

See more:

I was wondering if I can make function "strinsert" in this code more simple or shorter.

What I have tried:

int strinsert(char *dst, int len, const char *src, int offset)
{
  char leng[len];
  int i, j=0, x=0;
  if (strlen(src)+strlen(dst)>len-1 || offset>strlen(dst))
    return 1;
  else
  {
    for (i=0; strlen(dst)+strlen(src)>=i; i++)
    {
      if (i<offset)
      {
        leng[i]=dst[i];
        x++;
      }
      else if (j<strlen(src))
      {
        leng[i]=src[j];
        j++;
      }
      else if (j<strlen(src)+strlen(dst))
      {
        leng[i]=dst[x];
        x++;
      }
    }
  leng[strlen(leng)]='\0';
  strcpy(dst,leng);
  return 0;
  }
}

int main()
{
  char dst[100], src[100];
  int len, offset;
  scanf("%s %d %s %d", dst, &len, src, &offset);

  if (strinsert(dst, len, src, offset))
    printf("Failed\n");
   else
    printf("%s", dst);
  return 0;
}

Posted 28-Dec-17 1:10am

Member 13592362

Updated 28-Dec-17 2:07am

Add a Solution

Comments

Patrice T 28-Dec-17 7:26am

What is supposed to do the function?
Have examples input and output?

2 solutions

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

OriginalGriff · Answer 1 · 2017-12-28T01:32:00

Here's the fun bit: I don't like that code.
It's the last bit that says "NO!" to me:

strcpy(dst,leng);

leng is defined as being the size of the output - but there is no guarantee that dst has that much space - and since you are inserting a chunk of data in the middle of the original content of dst, it is almost certain that it will be longer than t was - which means it may well exceed the space allocated for it. If it does, your code will overwrite subsequent data.
A better approach would be to use malloc to allocate the output area and return it from your function instead of overwriting the original.

There's also the use of strlen without understanding what it does: it counts the characters up to the first null and returns that count.
So why do you use it to determine the location to put a terminating null?

leng[strlen(leng)]='\0';

By definition, there is already a null at that location...
It's also a slow operation - so using it twice each time you go round a loop is horribly inefficient, given that neither src nor dst get modified inside the loop.

I'd do it this way:
1) malloc enough space to hold the output.
2) Use memcpy to copy the first part of dst over.
3) Use memcpy to copy src over.
4) use memcpy to copy the end of dst over.
5) return the new string.

This also means that you can declare it as

char *strinsert(const char *dst, int len, const char *src, int offset)

Which means you can pass constant strings to the function. And it'll be a load quicker!

W∴ Balboos, *GHB* · Answer 2 · 2017-12-28T02:07:00

Aside from OriginalGriff's commentary, you request to shorten your code (and make it much more readable) can be accomplished by not repeating the same function calls, internally.

int srcLen = strlen(src);
int dstLen = strlen(src);

As a good rule-of-thumb guide, if you call the same function in a situation where its return value will not change between calls, calling it more than once is almost certainly less efficient and readable than assigning the return value to a symbol and using that.

Make function more simple

2 solutions

Solution 1

Solution 2

Add your solution here

Preview 0