So since I'm just new with learning C (this is the second day now), I'd be very happy if someone could review my code:
what I tried to do is:
- create a structure containing a string- and a length-property
- I want to create a function, that checks for the length of the string and sets the
length
property to the result - check, whether the result is correct with comparing to
strlen
of the original string - print the length of the string
This all works so far. Do you have any hints, what I could do better (or is ugly), what is wrong or where are any risks in my code?
this is my "helloworld.h" header file:
#include <stdio.h>
#include <stdlib.h>
struct SomeStringStruct
{
char* actual_string;
int length;
};
void calculate_length(struct SomeStringStruct*);
int get_string_length(char*);
and this is my "helloworld.c" source file:
#include <stdio.h>
#include <stdlib.h>
#include "helloworld.h"
int main(int argc, char* argv[])
{
char string[] = "This is a test";
char* ptrString = string;
printf("strlen ptrString: %lu\n", strlen(ptrString));
printf("get_string_length ptrString: %lu\n", get_string_length(ptrString));
return 0;
}
void calculate_length(struct SomeStringStruct* ptrAnyStringStruct)
{
ptrAnyStringStruct->length = 0;
while(ptrAnyStringStruct->actual_string[ptrAnyStringStruct->length] != '0円')
ptrAnyStringStruct->length++;
}
int get_string_length(char* string)
{
struct SomeStringStruct myString;
myString.actual_string = string;
calculate_length(&myString);
return myString.length;
};
1 Answer 1
Don't use Hungarian notation
The compiler doesn't need it and doesn't check it, and for a human it is all too easy to accidentily add the wrong prefix and get confused later. So no ptr
in front of pointer variables.
Array variables can be used as pointers.
In the following code:
char string[] = "This is a test";
char* ptrString = string;
You don't need the second variable at all. Just use string
directly.
Include the right header files
The function strlen()
is officially defined in <string.h>
, so make sure to #include
that at the top of your program. Sometimes your program works, maybe even without compiler warnings, but you cannot assume that it will keep working in the future or that you can port it to other systems if you don't.
Don't include headers you don't use
You include headers at the top of helloworld.h
that you are not using in that file. You should remove them.
Protect your header files from multiple inclusion
When you have a larger project, it can happen that your own header files need to #include
some of your other header files. It can then become unavoidable that a header files inadvertantly gets included twice. To prevent any errors from happening, use the following trick to allow a header file to be included multiple times:
#ifndef __HELLOWORLD_H__
#define __HELLOWORLD_H__
...the real contents of helloworld.h here...
#endif
Use better names
Variables and structs should have names that are clear and to the point. Naming something SomeStringStruct
is not to the point, it sounds vague and handwavy. Choose something better. The same goes for ptrAnyStringStruct
.
Use size_t
for lengths and sizes
The proper type to store the length of a buffer or a string is size_t
. If you read the manual page of strlen()
, you will also notice that it returns a value of type size_t
.
Use the correct format specifier for size_t
Use %zu
instead of %lu
when you are printing a variable of type size_t
.
If %zu
is not supported by your compiler, then cast the variable to the right type when printing it; for example with %lu
, cast it to long unsigned int
:
printf("string length: %lu\n", (long unsigned int)strlen(string));
Be aware though that on some systems (notably 64-bits Windows), long int
can be smaller than size_t
, and if you would ever have a string longer than can be represented in a long int
, the output will be incorrect.
-
\$\begingroup\$ wow, nice, there are a few things I didn't know (and I few things I forgot). Well, yes reinventing the wheel was wanted here, just for learning.
%zu
didn't work - it printedstrlen ptrString: zu
in my console \$\endgroup\$Matthias Burger– Matthias Burger2017年02月08日 08:41:19 +00:00Commented Feb 8, 2017 at 8:41 -
\$\begingroup\$ Okay, I read, that (since I'm using VS2013)
%zu
is not supported, it's C89.. \$\endgroup\$Matthias Burger– Matthias Burger2017年02月08日 09:00:02 +00:00Commented Feb 8, 2017 at 9:00