i dont know what im doing wrong in this program it works ok in online compilers but when im uploading it on my teacher's website that autorating programms
i get this message ERROR: INVALID MEMORY REFERENCE

thanks in advance
here is the code
// there is no problem in main bc it was given to us we had to create the functions//

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

char * readfromKeyboard()
{
char* s=NULL;
free(s);
s = realloc(s,20*sizeof(char));
if(s!=NULL)
{
fgets(s,20,stdin);
if(strlen(s)!=1)
s[strcspn(s,"\n")]='\0';
if(s[0]=='\0')
{
s = NULL;
}
else
{
s = realloc(s,strlen(s));
}
}
if(s[0]=='\n')
s[0]='\0';
return s;
} char*removeNonLatin(char*s2)
{ int i,j=0;
char *temp=NULL;
free(temp);
temp = realloc(temp,20*sizeof(char));

for(i=0;i<strlen(s2);i++)
{
if(s2[i]>=65 && s2[i]<=90)
{
temp[j] = s2[i]+32;
j++;
}
if(s2[i]>=97 && s2[i]<=122)
{
temp[j]= s2[i];
j++;
}
temp = realloc(temp,j*sizeof(char));
}
return temp;
}

int * getLetters(char* s3)
{ int *temp2=NULL;
int i,j,count=0;
free(temp2);
temp2 = realloc(temp2,26*sizeof(int));
for(i=0;i<26;i++)
temp2[i]=0;
if(s3!=NULL)
{
for(i=97;i<=122;i++)
{
count=0;
for(j=0;j<strlen(s3);j++)
{
if(s3[j]==i)
{
count++;
}
}
temp2[i-97]=count;
}
}
else
temp2[0]='\0';
return temp2;
}

void printLetters(int*letters)
{ int i,count=0,count2=0;
for (i=0;i<26;i++)
{
if(letters[i]>0)
{
count++;

}
}
for (i=0;i<26;i++)
{

if(letters[i]>0&&count!=0)
{
count2++;
if(count2==count)
{
printf("%c: %d\n",i+97,letters[i]);
}
else
{
printf("%c: %d, ",i+97,letters[i]);
}
}
}
}

void getMaxMin(int* letters,int *max ,int* min)
{ *max = 0;
*min = 0;
int i,meg=0,elax=27;

for(i=0;i<26;i++)
{
if (meg<letters[i])
{
meg = letters[i];
*max = i;
}
if (elax>letters[i])
{
elax = letters[i];
*min = i;
}
}
}

char *encode(int * letters)
{ char *stemp=NULL;
int i,j=0,k;
free(stemp);
stemp = realloc(stemp,20*sizeof(char));

for (i=0;i<26;i++)
{
if(letters[i]>0)
{
for(k=0;k<letters[i];k++)
{
stemp[j]= i+97;
j++;
}
}
}
stemp = realloc(stemp,j*sizeof(char));
return stemp;
}

int main()
{ char *s, *s2, *s3;
int *Letters,i,max,min;
s = readfromKeyboard();
while (s[0]!='\0')
{
printf ("[%s] -> ",s);
s2 = removeNonLatin(s);
printf ("[%s]\n",s2);
Letters = getLetters(s2);
printf("a: %d, z: %d\n",Letters[0],Letters[25]);
printLetters(Letters);
printLetters(Letters);
printLetters(Letters);
getMaxMin(Letters,&max,&min);
printf ("Max->%c[%d]\nMin->%c[%d]\n",'a'+max,Letters[max],'a'+min,Letters[min]);
s3 = encode(Letters);
printf ("%s\n",s3);
printf ("%s\n",s3);
printf ("%s\n",s3);
printf ("%s\n",s3);
s = readfromKeyboard();
}
return 0;
}

On 12/02/2022 23:31, kerif kerif wrote:
> i dont know what im doing wrong in this program it works ok in online compilers but when im uploading it on my teacher's website that autorating programms
> i get this message ERROR: INVALID MEMORY REFERENCE
>
> thanks in advance
> here is the code
> // there is no problem in main bc it was given to us we had to create the functions//
>
>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
>
> char * readfromKeyboard()
> {
>
> char* s=NULL;
> free(s);
> s = realloc(s,20*sizeof(char));
> if(s!=NULL)
> {
> fgets(s,20,stdin);
>
> if(strlen(s)!=1)
> s[strcspn(s,"\n")]='\0';
> if(s[0]=='\0')
> {
> s = NULL;
> }
> else
> {
> s = realloc(s,strlen(s));
> }
> }
> if(s[0]=='\n')
> s[0]='\0';
> return s;
> }
> char*removeNonLatin(char*s2)
> {
> int i,j=0;
> char *temp=NULL;
> free(temp);
> temp = realloc(temp,20*sizeof(char));
>
> for(i=0;i<strlen(s2);i++)
> {
> if(s2[i]>=65 && s2[i]<=90)
> {
> temp[j] = s2[i]+32;
> j++;
> }
> if(s2[i]>=97 && s2[i]<=122)
> {
> temp[j]= s2[i];
> j++;
> }
> temp = realloc(temp,j*sizeof(char));
> }
> return temp;
> }

I think that here, the new 'temp' string is not zero-terminated. It
might be an issue elsewhere too.

With certain compilers, the new strings generated tended to have garbage
at the end.

Although it didn't cause a memory error for me, it should be fixed first.

kerif kerif <keriftheofilos@gmail.com> writes:

> i dont know what im doing wrong in this program it works ok in online compilers but when im uploading it on my teacher's website that autorating programms
> i get this message ERROR: INVALID MEMORY REFERENCE

The autorating program is probably using different input than you are.

There are a bunch of things that look weird in this program, but I'll
just point out a few in the first function. I've reformatted it to show
the logic more clearly.

> char * readfromKeyboard()
> {
>
> char* s=NULL;
> free(s);
> s = realloc(s,20*sizeof(char));

These 3 lines are exactly the same as if you had said "s = malloc(20);"
But don't worry about that now; it isn't actually wrong.

> if(s!=NULL)
> {
> fgets(s,20,stdin);

This will get 20 characters. Can there be more than 19 characters in an
input line? If so, you will have problems. Can you see why?

>
> if(strlen(s)!=1)
> s[strcspn(s,"\n")]='\0';

What's special about a strlen of 1?

What happens if more than 20 characters are entered? You may not have a
null character *or* a newline character.

> if(s[0]=='\0')
> {
> s = NULL;

What happens to the memory that s was pointing to?

> }
> else
> {
> s = realloc(s,strlen(s));

Why do you do the realloc at all?

Think of the null character at the end of the string. Will it still be
in the range of memory that is allocated? [Hint: the answer is "no."]

> }
> }

> if(s[0]=='\n')
> s[0]='\0';

I'm really not sure what you think you are doing here. What happens if
'\n' is the first character read in? Figure out what happens if that is
the case.

> return s;
> }

Good luck.

kerif kerif <keriftheofilos@gmail.com> writes:

> i dont know what im doing wrong in this program it works ok in online
> compilers but when im uploading it on my teacher's website that
> autorating programms i get this message ERROR: INVALID MEMORY
> REFERENCE

I am just going to make some random remarks. Don't worry that there a
lot of them. We were all where you are at some time in the past...

> thanks in advance here is the code // there is no problem in main bc
> it was given to us we had to create the functions//

I would not go that far! What you mean is that no one is permitted to
point out anything wring in main!

> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
>
> char * readfromKeyboard()

For the last 30 years or so, C functions should be given what are called
prototypes. To make this a "modern" C function defintion write

char *readfromKeyboard(void)

Also, it's a bad name. It reads from the standard input stream which
make not be from a keyboard. But since main is given to you, you have
to used this name.

> {
>
> char* s=NULL;
> free(s);
> s = realloc(s,20*sizeof(char));

Why do you do is this way? The free is pointless and realloc on a null
pointer is just a complex way to write malloc.

char *s = malloc(20);

> if(s!=NULL)
> {
> fgets(s,20,stdin);
>
> if(strlen(s)!=1)
> s[strcspn(s,"\n")]='\0';

What's the point of the test? But kudos for using strcspn. It's a neat
way to replace the newline and it works even if there is no newline.

Note your indentation is messed up and I would like to see more spaces.
There's no need to pack the lines like that.

> if(s[0]=='\0')
> {
> s = NULL;

This looks odd. Were told to do this? A line with only a newline is
just an empty line I would have thought your code should return an
empty string.

I've not looked at the main function, and you should never return NULL.
Instead, an empty string is used to signal the end of the input.

> }
> else
> {
> s = realloc(s,strlen(s));
> }

This is wrong. The string needs strlen(s) + 1 bytes. But why realloc?
Were you told to do it? It is almost always better to simply live with
a few unused bytes.

> }
> if(s[0]=='\n')
> s[0]='\0';

s could be NULL here. But you don't need this code at all as far as I
can see (provided you fix a few things above).

> return s;

Having now read the main function you were given, it seems you should
never return NULL.

> }

> char*removeNonLatin(char*s2)

Why s2? That's an odd name.

> {
> int i,j=0;
> char *temp=NULL;
> free(temp);
> temp = realloc(temp,20*sizeof(char));

Again, char *s = malloc(20); seems preferable to me.

> for(i=0;i<strlen(s2);i++)

In modern C you can write

for (int i = 0; i < strlen(s2); i++)

But it's not a great idea to use strlen where it might be called many
times on a string that is not changing. Looping over the characters in
a string is almost always better written by stopping at the null:

for (int i = 0; s2[i]; i++)

(Put s2[i] != 0 or s2[i] != '\0' in there if you prefer.)

> {
> if(s2[i]>=65 && s2[i]<=90)
> {
> temp[j] = s2[i]+32;
> j++;
> }
> if(s2[i]>=97 && s2[i]<=122)

Why all the mysterious numbers? Yes, I know what they are, but why
write them as numbers at all? You can use 'A' or 'a' instead and
everyone will see what the code is doing.

But C has standard functions like isupper and islower for this. What's
more, you can use tolower to get rid of the 32.

> {
> temp[j]= s2[i];
> j++;
> }
> temp = realloc(temp,j*sizeof(char));

Again, the size is wrong. Again, why realloc at all? And why
reallocate every time round the loop?

By the way, sizeof(char) is always 1. For char arrays, just giving the
number of character needed is enough i.e. temp = realloc(temp, j);

But if you do want to include the type, a neat way to do that is this:

p = realloc(p, length * sizeof *p);

This reallocates the storage to include at least 'length' elements
no matter what the type of object p points to.

> }
> return temp;

As Bart has said, temp is not null terminated here.

> }
>
> int * getLetters(char* s3)

s3? Do you want to give every function unique names for the parameters?
That's not usual. Any function that processes a string could have a
parameter called 's' though something a bit longer might be preferable.

> {
> int *temp2=NULL;
> int i,j,count=0;
> free(temp2);
> temp2 = realloc(temp2,26*sizeof(int));

You know what I want to say here! Why the free? Why the realloc?

But temp2 is a bad name. It points to key, very much not temporary,
data. There are the letter counts, so call it that:

int *letterCounts = malloc(26 * sizeof *letterCounts);

> for(i=0;i<26;i++)
> temp2[i]=0;

Freefreetousemorespaces!

> if(s3!=NULL)
> {
> for(i=97;i<=122;i++)
> {
> count=0;
> for(j=0;j<strlen(s3);j++)
> {
> if(s3[j]==i)
> {
> count++;
> }
> }
> temp2[i-97]=count;
> }
> }
> else
> temp2[0]='\0';
> return temp2;
> }

Here, I am just going to suggest you think again. Do you think you
really need a loop inside a loop to count how often each letter appears?
You might think so, but you don't. Give it some more thought.

> void printLetters(int*letters)
> {
> int i,count=0,count2=0;
> for (i=0;i<26;i++)
> {
> if(letters[i]>0)
> {
> count++;
>
> }
> }
> for (i=0;i<26;i++)
> {
>
> if(letters[i]>0&&count!=0)
> {
> count2++;
> if(count2==count)
> {
> printf("%c: %d\n",i+97,letters[i]);
> }
> else
> {
> printf("%c: %d, ",i+97,letters[i]);
> }
> }
> }

I really don't know what this is supposed to do, but I'm running out of
time. It looks like there are lot of extraneous code here. I will skip
forwards...

> }
>
> void getMaxMin(int* letters,int *max ,int* min)
> {
> *max = 0;
> *min = 0;
> int i,meg=0,elax=27;
>
> for(i=0;i<26;i++)
> {
> if (meg<letters[i])
> {
> meg = letters[i];
> *max = i;
> }
> if (elax>letters[i])
> {
> elax = letters[i];
> *min = i;
> }
> }
> }
>
> char *encode(int * letters)
> {
> char *stemp=NULL;
> int i,j=0,k;
> free(stemp);
> stemp = realloc(stemp,20*sizeof(char));
>
> for (i=0;i<26;i++)
> {
> if(letters[i]>0)
> {
> for(k=0;k<letters[i];k++)
> {
> stemp[j]= i+97;
> j++;
> }
> }
> }
> stemp = realloc(stemp,j*sizeof(char));
> return stemp;
> }
>
>
> int main()
> {
> char *s, *s2, *s3;
> int *Letters,i,max,min;
> s = readfromKeyboard();
> while (s[0]!='\0')

This code (given to you) let's you know what readfromKeyboard should be
doing. It looks like your teacher expects the end of input to be
signalled by an empty string and that your should never return a NULL
pointer.

> {
> printf ("[%s] -> ",s);
> s2 = removeNonLatin(s);
> printf ("[%s]\n",s2);
> Letters = getLetters(s2);
> printf("a: %d, z: %d\n",Letters[0],Letters[25]);
> printLetters(Letters);
> printLetters(Letters);
> printLetters(Letters);
> getMaxMin(Letters,&max,&min);
> printf ("Max->%c[%d]\nMin->%c[%d]\n",'a'+max,Letters[max],'a'+min,Letters[min]);
> s3 = encode(Letters);
> printf ("%s\n",s3);
> printf ("%s\n",s3);
> printf ("%s\n",s3);
> printf ("%s\n",s3);
> s = readfromKeyboard();
> }
> return 0;

I have a lot of comments about this function but since it is your
teacher's code, you just have to use it!

> }

--
Ben.

Στις Κυριακή 13 Φεβρουαρίου 2022 στις 4:15:37 π.μ. UTC+2, ο χρήστης Ben Bacarisse έγραψε:
> kerif kerif <kerifth...@gmail.com> writes:

THANK YOU for the replies guys u helped me find all of my mistakes in the code

Ben i will take every advise u gave me
1) im gonna think a solution for the '' count each character'' without 2 loops
2)char * readfromKeyboard()
char * readfromKeyboard(void) what's the difference
3)u are right with the names i have to start making names that makes sense

On 13/02/2022 14:03, kerif kerif wrote:
> Στις Κυριακή 13 Φεβρουαρίου 2022 στις 4:15:37 π.μ. UTC+2, ο χρήστης Ben Bacarisse έγραψε:
>> kerif kerif <kerifth...@gmail.com> writes:
>
> THANK YOU for the replies guys u helped me find all of my mistakes in the code
>
> Ben i will take every advise u gave me
> 1) im gonna think a solution for the '' count each character'' without 2 loops
> 2)char * readfromKeyboard()
> char * readfromKeyboard(void) what's the difference

() means the parameter list is unknown and unchecked.

(void) means the parameter list is empty.

Take this call:

s = readfromKeyboard();

As you have it now, this can be legally compiled as well:

s = readfromKeyboard(1, 2.0, "three");

/Any/ set of arguments will work. This is dangerous in general as they
are unchecked, and if the program doesn't crash, if can still go quietly
wrong.

> 3)u are right with the names i have to start making names that makes sense

The 'readfromKeyboard' name is fine. While the function /does/ read from
stdin, that is the only way you can get fgets to read from the keyboard!

It depends on whether you see this as primarily interactive, so using
keyboard input, with optional redirection from a file; or primarily from
a redirected file.

On 12/02/2022 23:31, kerif kerif wrote:
> i dont know what im doing wrong in this program it works ok in online compilers but when im uploading it on my teacher's website that autorating programms
> i get this message ERROR: INVALID MEMORY REFERENCE
>
> thanks in advance
> here is the code
> // there is no problem in main bc it was given to us we had to create the functions//
>
>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
>
> char * readfromKeyboard()
> {
>
> char* s=NULL;
> free(s);
> s = realloc(s,20*sizeof(char));
> if(s!=NULL)
> {
> fgets(s,20,stdin);
>
> if(strlen(s)!=1)
> s[strcspn(s,"\n")]='\0';
> if(s[0]=='\0')
> {
> s = NULL;
> }
> else
> {
> s = realloc(s,strlen(s));
> }
> }
> if(s[0]=='\n')
> s[0]='\0';
> return s;
> }

I don't know if it's allowed, but I would keep this stuff simple. For
example, the above function can be written like this:

#define maxline 300

char * readfromKeyboard(void)
{ static char s[maxline];
fgets(s,maxline,stdin);
return s;
}

Although this gives a max line of 300 characters (298 excl newline and
terminator), and the result must be consumed before the next call.

This leaves the newline in place; since you can't change the main loop
(an empty line will be a string of {10, 0} not {0}), you need to add
logic to remove it if present.

(Or, just for the case of an empty line, return s+1 to skip the newline;
a terminating newline character can be removed in the other function.)

I'd simplify the other functions in the same way, at least until you
have a sane approach to allocating and deallocating short strings.

On 2/13/22 09:03, kerif kerif wrote:
> Στις Κυριακή 13 Φεβρουαρίου 2022 στις 4:15:37 π.μ. UTC+2, ο χρήστης
> Ben Bacarisse έγραψε:
>> kerif kerif <kerifth...@gmail.com> writes:
>
> THANK YOU for the replies guys u helped me find all of my mistakes in
> the code
> Ben i will take every advise u gave me 1) im gonna think a solution
> for the '' count each character'' without 2 loops
> 2)char * readfromKeyboard()
> char * readfromKeyboard(void) what's the difference 3)u are right with
> the names i have to start making names that makes sense

char * readfromKeyboard();

Is a K&R style declaration that specifies that readfromKeyboard() takes
an unspecified number of arguments, which might be 0. Passing the wrong
number of arguments to readfromKeyboard(), or passing arguments whose
promoted type isn't compatible withe the type of the corresponding
parameter has undefined behavior, which means that the C standard
imposes no requirements of any kind on the behavior of such a program.
Is there anything that a computer program could do, that you don't want
your program to do? If the behavior is undefined, an implementation can
translate your source code into a program which does it.

char * readfromKeyboard(void);

Is a function prototype declaration, which declares the number and type
of every function parameter. Those were introduced in C90, when C was
first standardized. This particular prototype specifies that
readfromKeyboard() takes no arguments. It's therefore a constraint
violation to pass any arguments to readfromKeyboard(). That means an
implementation is required to generate a diagnostic message at compile
time, which is a much nicer way to find out you've made such a mistake.

Bart <bc@freeuk.com> writes:

> On 13/02/2022 14:03, kerif kerif wrote:

>> 3)u are right with the names i have to start making names that makes sense
>
> The 'readfromKeyboard' name is fine. While the function /does/ read
> from stdin, that is the only way you can get fgets to read from the
> keyboard!

The function could arrange for fgets to read data from "the keyboard" so
as to merit the name, but this function just get data from somewhere.
It is one of the most significant abstractions provided by modern
programming systems (languages and OSes) that the program does not need
to know the source of the data.

> It depends on whether you see this as primarily interactive, so using
> keyboard input, with optional redirection from a file; or primarily
> from a redirected file.

The name of the function should not reflect how the programmer sees the
use of the program but rather it should reflect what the function
actually does.

People staring out on the programming journey should be helped to see
that, for a huge number of programs, the input is just the input and the
output is just the output. All to often I see programs that open
"input.txt" and maybe "output.txt" which does nothing but make the
program hard to test[1]. A gentle nudge towards writing code that does
not care about data sources and sinks is, in my opinion, the more
helpful kind of reply.

What's more, I'd put money on the fact that the marking system to which
the OP has to post the program does not have a person at a keyboard
typing input! The program will be run with stdin and stdout connected
in such a way that a number of input cases can be checked. And the OP
should learn to do that as soon as possible as well because that's what
makes programmer testing simple.

[1] Fortunately in Unix-like systems one can usually get round this with
symbolic links and virtual devices like /dev/stdin.

--
Ben.

kerif kerif <keriftheofilos@gmail.com> writes:

> Στις Κυριακή 13 Φεβρουαρίου 2022 στις 4:15:37 π.μ. UTC+2, ο χρήστης Ben Bacarisse έγραψε:
>> kerif kerif <kerifth...@gmail.com> writes:
>
> THANK YOU for the replies guys u helped me find all of my mistakes in
> the code

You are welcome.

> Ben i will take every advise u gave me
> 1) im gonna think a solution for the '' count each character'' without
> 2 loops
> 2)char * readfromKeyboard()
> char * readfromKeyboard(void) what's the difference

char *readfromKeyboard(); is an old-style C declaration. All functions
were written this way even when they did take arguments (trust me, you
really don't want to know how it all worked back then!) but because C
has a very strong tradition of not breaking old code, this method is
still permitted.

To say that a function takes no arguments, you need to write 'void' in
the brackets. If you do that, a function call like

readfromKeyboard(42);

can (and should) be reported as an error. Without the 'void', anything
is permitted and it becomes very hard to the compiler to help with error
messages.

(This is just a duplicate of what Bart said, but I wanted to say a bit
about the history.)

--
Ben.

On 13/02/2022 16:19, Ben Bacarisse wrote:
> Bart <bc@freeuk.com> writes:
>
>> On 13/02/2022 14:03, kerif kerif wrote:
>
>>> 3)u are right with the names i have to start making names that makes sense
>>
>> The 'readfromKeyboard' name is fine. While the function /does/ read
>> from stdin, that is the only way you can get fgets to read from the
>> keyboard!
>
> The function could arrange for fgets to read data from "the keyboard" so
> as to merit the name, but this function just get data from somewhere.
> It is one of the most significant abstractions provided by modern
> programming systems (languages and OSes) that the program does not need
> to know the source of the data.
>
>> It depends on whether you see this as primarily interactive, so using
>> keyboard input, with optional redirection from a file; or primarily
>> from a redirected file.
>
> The name of the function should not reflect how the programmer sees the
> use of the program but rather it should reflect what the function
> actually does.
>
> People staring out on the programming journey should be helped to see
> that, for a huge number of programs, the input is just the input and the
> output is just the output. All to often I see programs that open
> "input.txt" and maybe "output.txt" which does nothing but make the
> program hard to test[1]. A gentle nudge towards writing code that does
> not care about data sources and sinks is, in my opinion, the more
> helpful kind of reply.
>
> What's more, I'd put money on the fact that the marking system to which
> the OP has to post the program does not have a person at a keyboard
> typing input!

Yet they chose the name readfromKeyboard!

> The program will be run with stdin and stdout connected
> in such a way that a number of input cases can be checked. And the OP
> should learn to do that as soon as possible as well because that's what
> makes programmer testing simple.

The simplest way to start testing for something like this is to hard
code a test input:

char* s = "abc def";

Then the program can be run repeatedly with this until it works, at
which point it is easy to change, or add a new one, or create a list of
such test strings.

As it as, I can imagine many might actually be keying in the same test
string each time, in a program that doesn't even have a prompt.

> output is just the output. All to often I see programs that open
> "input.txt" and maybe "output.txt" which does nothing but make the
> program hard to test[1]. A gentle nudge towards writing code that does

For some kinds of input, this makes things easier with a hardcoded
filename instead of a hardcoded input string:

* Always just run prog.exe or ./prog

* No need to specify any input, nor remember what the input should be.

Moving to an input file specified from a command line (or relying on
input piped into stdin) is for a more advanced program IMO.

Bart <bc@freeuk.com> writes:

> On 13/02/2022 16:19, Ben Bacarisse wrote:
>> Bart <bc@freeuk.com> writes:
>>
>>> On 13/02/2022 14:03, kerif kerif wrote:
>>
>>>> 3)u are right with the names i have to start making names that makes sense
>>>
>>> The 'readfromKeyboard' name is fine. While the function /does/ read
>>> from stdin, that is the only way you can get fgets to read from the
>>> keyboard!
>> The function could arrange for fgets to read data from "the keyboard" so
>> as to merit the name, but this function just get data from somewhere.
>> It is one of the most significant abstractions provided by modern
>> programming systems (languages and OSes) that the program does not need
>> to know the source of the data.
>>
>>> It depends on whether you see this as primarily interactive, so using
>>> keyboard input, with optional redirection from a file; or primarily
>>> from a redirected file.
>> The name of the function should not reflect how the programmer sees the
>> use of the program but rather it should reflect what the function
>> actually does.
>> People staring out on the programming journey should be helped to see
>> that, for a huge number of programs, the input is just the input and the
>> output is just the output. All to often I see programs that open
>> "input.txt" and maybe "output.txt" which does nothing but make the
>> program hard to test[1]. A gentle nudge towards writing code that does
>> not care about data sources and sinks is, in my opinion, the more
>> helpful kind of reply.
>> What's more, I'd put money on the fact that the marking system to which
>> the OP has to post the program does not have a person at a keyboard
>> typing input!
>
> Yet they chose the name readfromKeyboard!

Yes, but you're fine with it.

In my opinion its not a good choice, and other choices like "getLetters"
and the cryptic "encode", are arguably even worse. And then there's the
declaration of an unused variable, the lack of indentation and the odd
choice that an empty line signals the end of the input. And what's with
the apparently pointless duplicate printf calls?

It's possible that the students were given a sketch of main and this is
not really the tutor's code. But if it is, it does not bode well for
the quality of the exercise.

>> The program will be run with stdin and stdout connected
>> in such a way that a number of input cases can be checked. And the OP
>> should learn to do that as soon as possible as well because that's what
>> makes programmer testing simple.
>
> The simplest way to start testing for something like this is to hard
> code a test input:
>
> char* s = "abc def";

The OP needed help with the input function. That's the "something like
this" I was talking about.

But for a version that does not yet do any input, a hard-coded string
will work for a run or two. But I would advise using a command-line
argument, giving the students the code if the course had not yet reached
the point where an explanation of how to do this was possible:

int main(int argc, char **argv)
{ if (argc > 1) {
char *s = argv[1];
...
}
}

The point being to instil these habits as early as possible.

> Then the program can be run repeatedly with this until it works, at
> which point it is easy to change, or add a new one, or create a list
> of such test strings.
>
> As it as, I can imagine many might actually be keying in the same test
> string each time, in a program that doesn't even have a prompt.

I fear you may be right, but the tutor should have explained how to
avoid that. In bash you can just type

./my_program <<<"abc def"

and up arrow, twice, (probably) gets you the compile command. For
learning programming to be fun, you need to take as much of the tedious
clerical work away.

>> output is just the output. All to often I see programs that open
>> "input.txt" and maybe "output.txt" which does nothing but make the
>> program hard to test[1]. A gentle nudge towards writing code that does
>
> For some kinds of input, this makes things easier with a hardcoded
> filename instead of a hardcoded input string:
>
> * Always just run prog.exe or ./prog
>
> * No need to specify any input, nor remember what the input should be.

I disagree. For one thing, you want to build up a set of test cases so
there's no remembering involved. Editing the one file (remembering to
copy the old one if it was an interesting case) and having to "type
ouput.txt" all the time is just busy work.

> Moving to an input file specified from a command line (or relying on
> input piped into stdin) is for a more advanced program IMO.

Eh? I think that's backwards. Using fopen and handling the errors
comes after getting the hang of just reading stdin. And running

./prog <<<"This string works"

and/or

./prog <fail_case1

is hardly advanced. It's way simpler than having a hard-wired file name
in the code.

--
Ben.

On Sunday, 13 February 2022 at 21:37:45 UTC, Ben Bacarisse wrote:
>
> The OP needed help with the input function. That's the "something like
> this" I was talking about.
>
> But for a version that does not yet do any input, a hard-coded string
> will work for a run or two. But I would advise using a command-line
> argument, giving the students the code if the course had not yet reached
> the point where an explanation of how to do this was possible:
>
> int main(int argc, char **argv)
> {
> if (argc > 1) {
> char *s = argv[1];
> ...
> }
> }
>
In real programs you seldom do a little dialogue with the user via stdin and stdout.
In beginner exercises, it is common. Because C has extremely simple IO
primitives, making the input routine really robust would often take the beginner
into considerations way beyond the exercise. fgets() can fail because of a device error,
for example.
Passing arguments on the commandline avoids a lot of these problems. But it is
harder to pass lines of text. Normally in a real program, lines of text would be in a
data file on disk.

Malcolm McLean <malcolm.arthur.mclean@gmail.com> writes:

> On Sunday, 13 February 2022 at 21:37:45 UTC, Ben Bacarisse wrote:
>>
>> The OP needed help with the input function. That's the "something like
>> this" I was talking about.
>>
>> But for a version that does not yet do any input, a hard-coded string
>> will work for a run or two. But I would advise using a command-line
>> argument, giving the students the code if the course had not yet reached
>> the point where an explanation of how to do this was possible:
>>
>> int main(int argc, char **argv)
>> {
>> if (argc > 1) {
>> char *s = argv[1];
>> ...
>> }
>> }
>>
> In real programs you seldom do a little dialogue with the user via
> stdin and stdout.

Hmm... I was advocating a learning plan that moved away from the
program having any dialogue with a user as quickly as possible. But I
think you are maybe using the term "dialogue with the user" to mean just
reading stdin and writing stdout. If so, then I agree that it's a
rather specialised class of program that does this. But it's a very
important class, none the less.

> In beginner exercises, it is common.

So that's why the discussion came up here. But even so, it is
depressingly common to see beginner exercises where the solution
explicitly opens input.txt (say) and reads that.

> Because C has extremely simple IO primitives, making the input routine
> really robust would often take the beginner into considerations way
> beyond the exercise. fgets() can fail because of a device error, for
> example.

I wouldn't call C's IO extremely simple. Extremely simple IO primitives
would be, to my mind, like those in AWK for example. I would bet
(though I've not studied many) that almost every modern programming
language has an input function simpler that fgets.

(I think this may just be words again. You might be using simple to
mean simplistic.)

> Passing arguments on the commandline avoids a lot of these
> problems.

Right, but you often don't want to avoid these problems, for example
when the whole point of the exercise is to learn about C's IO.

> But it is harder to pass lines of text.

Yes, a bit harder. If I can avoid ' characters I just start with a '
and type away, ending with another '.

> Normally in a real
> program, lines of text would be in a data file on disk.

And you can pass that as a command line argument as well if it suits
your testing regime:

./prog "$(cat data-file)"

--
Ben.

On Monday, 14 February 2022 at 00:52:46 UTC, Ben Bacarisse wrote:
> Malcolm McLean <malcolm.ar...@gmail.com> writes:

> > Because C has extremely simple IO primitives, making the input routine
> > really robust would often take the beginner into considerations way
> > beyond the exercise. fgets() can fail because of a device error, for
> > example.
> I wouldn't call C's IO extremely simple. Extremely simple IO primitives
> would be, to my mind, like those in AWK for example. I would bet
> (though I've not studied many) that almost every modern programming
> language has an input function simpler that fgets.
>
> (I think this may just be words again. You might be using simple to
> mean simplistic.)
>
Yes, but almost every modern programming language also has a function
which ships as standard which does the same job as fgets(), but is easier
to use because it does more internally. The obvious improvement that almost
every language makes is to allocate the memory for the line dynamically.
C doesn't do that do avoid making the stdio functions depend on memory
allocation functions, which makes providing a standard library for small
systems easier.

Lowell Gilbert <lgusenet@be-well.ilk.org> writes:

> kerif kerif <keriftheofilos@gmail.com> writes:
>
>> i dont know what im doing wrong in this program it works ok in online
>> compilers but when im uploading it on my teacher's website that
>> autorating programms
>> i get this message ERROR: INVALID MEMORY REFERENCE
>
> The autorating program is probably using different input than you are.
>
> There are a bunch of things that look weird in this program, but
> I'll just point out a few in the first function. I've reformatted
> it to show the logic more clearly.
>
>> char * readfromKeyboard()
>> {
>>
>> char* s=NULL;
>> free(s);
>> s = realloc(s,20*sizeof(char));
>
> These 3 lines are exactly the same as if you had said
> "s = malloc(20);" But don't worry about that now; it
> isn't actually wrong.
>
>> if(s!=NULL)
>> {
>> fgets(s,20,stdin);
>
> This will get 20 characters. Can there be more than 19 characters
> in an input line? If so, you will have problems. Can you see why?

This call to fgets() will read at most 19 characters. Assuming
the return value is not null, the memory pointed to by s might
or might not contain a \n, but it will always contain a null
character.

>> if(strlen(s)!=1)
>> s[strcspn(s,"\n")]='\0';
>
> What's special about a strlen of 1?
>
> What happens if more than 20 characters are entered? You may not
> have a null character *or* a newline character. [...]

In N1570, section 7.21.7.2, describing the fgets function, says
in paragraph 2:

The fgets function reads at most one less than the number
of characters specified by n from the stream pointed to by
stream into the array pointed to by s. No additional
characters are read after a new-line character (which is
retained) or after end-of-file. A null character is written
immediately after the last character read into the array.

There should be a test that the return value of fgets() is not
null, but as long as the return value is not null it is safe
to rely on the array having a null character.