Άσκηση στην C

alexc · 27 Μαρτίου 2013

Καλημέρα! Σας παραθέτω κατευθείαν την άσκηση που προσπαθώ να λύσω και δεν μπορώ να καταλάβω που στο καλό είναι το λάθος μου:

Να υλοποιηθεί συνάρτηση με όνομα Myconcat() η οποία θα δέχεται για ορίσματα

της δύο συμβολοσειρές S1 και S2. Η συνάρτηση θα πρέπει να δημιουργεί δυναμικά

και να επιστρεφει μια νέα σμβολοσειρά η οποία θα περιέχει τις S1 και S2

χωρισμένες με ένα χαρακτήρα αλλαγής γραμμής.

Και ο κώδικας μου:

char* myconcat(char *s1,char* s2)
{
	char *s;
	int i;
	s=(char*)malloc((strlen(s1)+strlen(s2)+1)*sizeof(char));
	printf("%d",strlen(s));
	for (i=0;i<strlen(s1);i++)
	   s[i]=s1[i];
	s[i]='\n';
	for (i=strlen(s1)+1;i<strlen(s);i++)
	   s[i]=s2[i];
	s[i]='\0';
	return s;      
}

main()
{
	char sy1[20],sy2[20],*s1,*s2,*newsyb;
	int i;
	
	printf("Give me the first name: ");
	gets(sy1);
	s1=(char*)malloc(strlen(sy1)+1);
	strcpy(s1,sy1);
	fflush(stdin);
	printf("Give me the second name: ");
	gets(sy2);
	s2=(char*)malloc(strlen(sy2)+1);
	strcpy(s2,sy2);
	newsyb=myconcat(s1,s2);
	for (i=0;i<strlen(newsyb);i++)
	   printf("%c",newsyb[i]);
	system("pause");   
}

Το πρόβλημα που εντόπισα είναι στο μέγεθος της s μέσα στην συνάρτηση το οποίο για κάποιο περίεργο λόγο είναι πάντα 30.Κατά τα άλλα πιστεύω είμαι σωστός σε όλα και δεν μπορώ να καταλάβω γιατί γίενται αυτό.

defacer · 27 Μαρτίου 2013

Γιατί δεν απλοποιείς λίγο τα πράγματα πρώτα;

Δεν έχει νόημα να κάνεις strcpy από τους buffers σε νέους buffers.
Μέσα στην myconcat γιατί δε χρησιμοποιείς stcpy() αντί για manual loops?

Επίσης έχεις ένα bug και ένα πταίσμα:

Δεν κάνεις malloc αρκετή μνήμη στη myconcat. Θέλεις strlen() + strlen() + 2 (+1 για το \n και ακόμα +1 για το \0 στο τέλος).
Μην κάνεις cast την επιστρεφόμενη τιμή της malloc.

Τέλος, δεν έχει κανένα νόημα να κάνεις printf("%d",strlen(s)) μιας και δεν ξέρεις ποιά είναι τα περιεχόμενα του s. Αυτή η κλήση της strlen βασικά οδηγεί σε undefined behavior, οπότε όχι απλά μπορεί να δεις άσχετο νούμερο αλλά μπορεί (θεωρητικά) να κάνει crash, να σου φορμάρει το σκληρό, κλπ κλπ.

Η strlen() θεωρεί δεδομένο πως θα βρει κάπου ένα \0. Η malloc() δεν υπόσχεται τίποτα για τα περιεχόμενα της μνήμης που σου παραχωρεί, άρα το δεδομένο δεν ισχύει.

javavall · 27 Μαρτίου 2013

Δες και την strcat ..

ωπ άκυρο....δε διάβασα καλά την εκφώνηση

alexc · 27 Μαρτίου 2013

Ευχαριστώ για τις συμβουλές σου defacer τελικά το πρόβλημα στην υλοποίηση μου είναι το strlen(s) το οποίο είχα ξεχάσει πώς ακριβώς λειτουργεί. Δεν κατάλαβα ομως τα σημεία που μου είπες "Δεν έχει νόημα να κάνεις strcpy από τους buffers σε νέους buffers.".Τί ακριβώς εννοείς?? Επίσης προσπάθησα να κάνω μια υλοποίση μόνο με strcpy() μέσα στην συνάρτηση αλλά δεν μπόρεσα καθώς σε ένα σημείο που πρέπει να βάλει την δεύτερη συμβολοσειρά στην s πώς ακριβώς υλοποιείται αυτό με strcpy()?? Προσπάθησα να κ΄ναω κάι τέτοιο όμως δεν δούλεψε κάτι που περίμενα κιόλας:

strcpy(s[strlen(s1)+1],s2);

imitheos · 27 Μαρτίου 2013

Καλημέρα! Σας παραθέτω κατευθείαν την άσκηση που προσπαθώ να λύσω και δεν μπορώ να καταλάβω που στο καλό είναι το λάθος μου:

Να υλοποιηθεί συνάρτηση με όνομα Myconcat() η οποία θα δέχεται για ορίσματα

της δύο συμβολοσειρές S1 και S2. Η συνάρτηση θα πρέπει να δημιουργεί δυναμικά

και να επιστρεφει μια νέα σμβολοσειρά η οποία θα περιέχει τις S1 και S2

χωρισμένες με ένα χαρακτήρα αλλαγής γραμμής.

Και ο κώδικας μου:
char* myconcat(char *s1,char* s2)
{
	char *s;
	int i;
	s=(char*)malloc((strlen(s1)+strlen(s2)+1)*sizeof(char));
	printf("%d",strlen(s));
	for (i=0;i<strlen(s1);i++)
	   s[i]=s1[i];
	s[i]='\n';
	for (i=strlen(s1)+1;i<strlen(s);i++)
	   s[i]=s2[i];
	s[i]='\0';
	return s;      
}

main()
{
	char sy1[20],sy2[20],*s1,*s2,*newsyb;
	int i;
	
	printf("Give me the first name: ");
	gets(sy1);
	s1=(char*)malloc(strlen(sy1)+1);
	strcpy(s1,sy1);
	fflush(stdin);
	printf("Give me the second name: ");
	gets(sy2);
	s2=(char*)malloc(strlen(sy2)+1);
	strcpy(s2,sy2);
	newsyb=myconcat(s1,s2);
	for (i=0;i<strlen(newsyb);i++)
	   printf("%c",newsyb[i]);
	system("pause");   
}
Το πρόβλημα που εντόπισα είναι στο μέγεθος της s μέσα στην συνάρτηση το οποίο για κάποιο περίεργο λόγο είναι πάντα 30.Κατά τα άλλα πιστεύω είμαι σωστός σε όλα και δεν μπορώ να καταλάβω γιατί γίενται αυτό.

Αν έχετε μάθει το const, εφόσον η συνάρτησή σου δεν πειράζει τα s1 και s2, μπορείς στη δήλωση της να τα ορίσεις ως const char *.

Μην κάνεις cast το αποτέλεσμα της malloc. Δεν χρειάζεται (εκτός αν χρησιμοποιείς c++ compiler) και μπορεί (υπό σπάνιες συνθήκες) να προκαλέσει προβλήματα. (το είπε και ο defacer)

Μην πολλαπλασιάζεις με sizeof(char) γιατί εγγυημένα είναι πάντα 1 οπότε δεν χρειάζεται και πρόσεξε το μέγεθος της malloc όπως σου είπε και ο defacer.

Αν έχετε μάθει τη γραφή *s = τάδε (αντί για s = τάδε) μπορείς να χρησιμοποιήσεις αυτή για να είναι πιο όμορφοι οι βρόχοι (ειδικά ο 2ος που έχεις strlen(s1)+1. Επίσης η συνθήκη τερματισμού στο δεύτερο βρόχο γιατί είναι strlen(s) και όχι strlen(s2) ?

Στην main γιατί κάνεις copy τα syΧ σε sΧ και μετά καλείς την συνάρτηση με αυτά ? Γιατί δεν την καλείς κατευθείαν ως myconcat(sy1,sy2) ?

Στο τέλος της main γιατί κάνεις ένα βρόχο με printf("%c") και δεν έχεις κατευθείαν ένα printf("%s") ?

Μην χρησιμοποιείς gets. Το fflush έχει εγγυημένη λειτουργία μόνο σε output streams οπότε η εντολή fflush(stdin) δεν είναι δόκιμη. Αντί για system("pause") καλύτερα να βάζεις getchar().

Edit: Δήλωνε την main ως "int main(void)".

albNik · 27 Μαρτίου 2013

Το θέμα είναι χωρις τις έτοιμες strcpy ,strlen ... αλλιως γράψε

char* myconcat(char *s1,char* s2)
{

     return strcat(s1,s2);

}

imitheos · 27 Μαρτίου 2013

Το θέμα είναι χωρις να τις έτοιμες strcpy ,strlen ... αλλιως γράψε
char* myconcat(char *s1,char* s2)
{

     return strcat(s1,s2);

}

#define myconcat strcat

defacer · 27 Μαρτίου 2013

Μην πολλαπλασιάζεις με sizeof(char) γιατί εγγυημένα είναι πάντα 1 οπότε δεν χρειάζεται και πρόσεξε το μέγεθος της malloc όπως σου είπε και ο defacer.

Εμένα πάντως μου αρέσει να το βλέπω στο source, γιατί αν δεν το δεις τότε λες "ώπα εδώ λείπει το μέγεθος, τι data type κάνουμε allocate είπαμε; α char, οπότε εντάξει". Σα τζάμπα mental overhead το βλέπω. Έτσι κι αλλιώς είναι compile time constant οπότε είτε το βάλεις είτε όχι για τον compiler δεν κάνει καμία διαφορά, άρα το κριτήριο θα πρέπει να είναι τι διαβάζεται ευκολότερα.

Στην main γιατί κάνεις copy τα syΧ σε sΧ και μετά καλείς την συνάρτηση με αυτά ? Γιατί δεν την καλείς κατευθείαν ως myconcat(sy1,sy2) ?

@alexc: Αυτό εννοούσα λέγοντας από buffers σε νέους buffers.

Κατά τα άλλα η Ηρακλάρα καλά τα λέει και με περισσότερη προσοχή στη λεπτομέρεια απο μένα.

Το θέμα είναι χωρις τις έτοιμες strcpy ,strlen ... αλλιως γράψε
char* myconcat(char *s1,char* s2)
{
     return strcat(s1,s2);
}

Πρέπει να μπει ένα \n στη μέση. Επιπλέον αυτό προϋποθέτει πως έχεις αρκετό χώρο στο s1 για να κολλήσεις το s2, πράγμα που δεν ισχύει γενικά. Και τέλος μια function που έχει side effects (μεταβάλλει το πρώτο όρισμα) όπως η παραπάνω γενικά είναι λιγότερο επιθυμητή από μία που δεν έχει.

#define myconcat strcat

ΠΙΣΩ ΚΑΙ ΣΑΣ ΕΦΑΓΑ ΟΧΤΡΟΙ! ΤΑ MACRO ΘΑ ΤΑ ΒΑΛΩ ΠΑΝΩ ΣΤΙΣ ΤΑΦΟΠΛΑΚΕΣ ΣΑΣ!

char (* const mystrcat) (char *, const char *) = strcat;

albNik · 27 Μαρτίου 2013

Ουτε εγώ διάβασα καλα την εκφώνηση

akisk · 27 Μαρτίου 2013

Εκτός των άλλων που ειπώθηκαν, υπάρχει σίγουρο σφάλμα εδώ:

for (i=strlen(s1)+1;i<strlen(s);i++)
	   s[i]=s2[i];

To string s2 από την αρχή δεν έπρεπε να το διαβάσεις;

migf1 · 27 Μαρτίου 2013

Μετά από τις παρατηρήσεις των άλλων παιδιών, consider και κάτι σαν αυτό...

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

/* --------------------------------------------- */
char *myConcat( const char *s1, const char *s2 )
{
	char glue = '\n';
	size_t size = 0;
	size_t s1len = 0, s2len = 0;
	char *ret = NULL;

	/* sanity checks (they may be improved for more explicit cases) */
	if ( !s1 || !s2 )
		return NULL;

	/* allocate zero'ed mem for the new c-string to be returned */
	s1len = strlen(s1);
	s2len = strlen(s2);
	size = s1len + s2len + 1 + 1;		/* +1 for glue, +1 for '\0' */
	if ( NULL == (ret = calloc(size,1)) )
		return NULL;

	memcpy(ret, s1, s1len);			/* copy s1 to ret */
	ret[s1len] = glue;			/* append glue to ret */
	memcpy( &ret[++s1len], s2, s2len);	/* append s2 to ret */

	return ret;
}

/* --------------------------------------------- */
int main( void )
{
	char *s1 = "abc";
	char *s2 = "defg";
	char *s = NULL;

	if ( NULL != (s = myConcat(s1,s2)) ) {
		puts(s);
		free(s);
	}

	system( "pause" );   /* Windows only */
	return 0;
}

imitheos · 27 Μαρτίου 2013

	memcpy(ret, s1, s1len);			/* copy s1 to ret */
	ret[s1len] = glue;			/* append glue to ret */
	memcpy( &ret[++s1len], s2, s2len);	/* append s2 to ret */

Δεν έχουμε την εκφώνηση της άσκησης οπότε το λέω με επιφύλαξη αλλά αν ήταν να επιτρέπεται η memcpy δεν θα επιτρεπόταν και η strcat ? Υποθέτω ότι το ζητούμενο είναι να το κάνουν με βρόχο και ανάθεση όπως το αρχικό μήνυμα.

migf1 · 27 Μαρτίου 2013

...

Επίσης προσπάθησα να κάνω μια υλοποίση μόνο με strcpy() μέσα στην συνάρτηση αλλά δεν μπόρεσα καθώς σε ένα σημείο που πρέπει να βάλει την δεύτερη συμβολοσειρά στην s πώς ακριβώς υλοποιείται αυτό με strcpy()?? Προσπάθησα να κ΄ναω κάι τέτοιο όμως δεν δούλεψε κάτι που περίμενα κιόλας: [/font][/color]
strcpy(s[strlen(s1)+1],s2);

Το μόνο που σου λείπει για να δουλέψει το παραπάνω είναι ο addressof operator στο s, δηλαδή το & ...

strcpy( &s[strlen(s1)+1], s2 );

Πιο αναλυτικά, η strcpy() αναμένει char * ως όρισμα, αλλά το s[whatever] είναι σκέτο char, για αυτό δεν σου δούλεψε.

Από εκεί και πέρα, άσχετα με το παραπάνω ως μεμονωμένη γραμμή, έχε υπόψη σου πως το strlen() διατρέχει κάθε φορά όλο το όρισμά της, χαρακτήρα προς χαρακτήρα. Για αυτό αποφεύγουμε να χρησιμοποιούμε strlen() συνεχώς, και προτιμάμε να την καλέσουμε μια μόνο φορά, αποθηκεύοντας την τιμή επιστροφή της σε μια άλλη μεταβλητή τύπου size_t, την οποία και χρησιμοποιούμε μετέπειτα όπου την χρειαζόμαστε χωρίς έξτρα κόστος καθυστέρησης.

Το επισημαίνω γιατί στον κώδικά σου. π.χ. αυτό εδώ:

	for (i=0; i < strlen(s1); i++)
	   s[i] = s1[i];

μετράει και ξανα μετράει το μήκος του s1 σε κάθε επανάληψη του loop. Ενώ π.χ. κάτι σαν αυτό εδώ:

	size_t s1len = strlen(s1);
	for (size_t i=0; i < s1len; i++)
	   s[i] = s1[i];

μετράει το μήκος του s1 μονάχα μια φορά, και κατόπιν δεν το ξανα υπολογίζει

@imitheos:

Είδα πως ο φίλος χρησιμοποίησε εξαρχής την strlen() καθώς και ότι κατόπιν έγραψε πως επιχείρησε και με strcpy(), οπότε θεώρησα πως δεν υπάρχει περιορισμός (δεν αναφέρει και τίποτα σχετικό στην εκφώνηση που παρουσίασε).

Σχετικά με την memcpy() έναντι της strcat(), είναι κατά κανόνα ταχύτερη, χωρίς καν να χρειάζεται ενεργοποίηση των optimization flags των compilers. Μπορεί πάντως ο φίλος να την αντικαταστήσει με την strcpy() αν το επιθυμεί (αν και νομίζω πως η memcpy() είναι και πάλι ταχύτερη, αλλά δεν το θυμάμαι σίγουρα... και βαριέμαι τώρα να το ψάξω).

alexc · 27 Μαρτίου 2013

Ευχαριστώ πολύ για τις απαντήσεις σας και για τις συμβουλές σου @migf1.

Με βοηθήσατε πάρα πολύ!

migf1 · 27 Μαρτίου 2013

Παρακαλώ, καλή συνέχεια

Σύνδεση

Άσκηση στην C

Προτεινόμενες αναρτήσεις

alexc

defacer

javavall

alexc

imitheos

albNik

imitheos

defacer

albNik

akisk

migf1

imitheos

migf1

alexc

migf1

Δημιουργήστε ένα λογαριασμό ή συνδεθείτε για να σχολιάσετε

Δημιουργία λογαριασμού

Σύνδεση

Σύνδεση