Вот посмотрите плиз мою функцию для кокантенации 2х строк

[QUOTE=RedHeart]
unsigned size_s1=strlen(s1);
unsigned size_s2=strlen(s2);
unsigned i=(strlen(s1)+strlen(s2)+1);
[/QUOTE]
Какая то ненужная избыточность вызовов strlen().
Размеры уже известны можно исспользовать size_s1 + size_s2 + 1

[QUOTE=RedHeart]
char *buf=new char ;
[/QUOTE]
Можно было бы конечно вместо new исспользовать realloc(s1, i), раз уж строки вида char *. Ну да ладно, это личное дело и смешивать с new не стоит.
[QUOTE=RedHeart]
char ch=NULL;
[/QUOTE]
NULL - это конечно 0, но всетаки логически он применяется как нулевой указатель. Тоесть - указатель равный нулю. К простым числовым/символьным переменным его применять логически некоректно. Хотя реально конечно разницы нету.

[QUOTE=RedHeart]
if(j<size_s1) ch=*s1++;
else ch=*s2++;
buf[j]=ch;
[/QUOTE]
Тут бы можно было обойтись одним buf[j], без ch =)

[QUOTE=RedHeart]
s1=buf;
[/QUOTE]
А вот тут уже ошибка. Память выделенная на которую указывал s1 не освобождается, а в s1 присваивается указатель на другую область памяти. В итоге старый указатель и его область памяти теряются.

Ну и в общем. Выделение памяти всегда немного неправильно. Всетаки памяти под обе строки может быть вполне достаточно в s1, поскольку строки ограничиваются нулем в конце и могут знимать далеко не всю память выделенную в указателе. Более того - логично требовать, чтобы перед вызовом этой функции в s1 было достаточно место, чтобы уместились обе строки. Иначе вызов считать некорректным с последствиями ввиде ошибок доступа к памяти =)

Aks, на счет того что память выделенная на которую указывает s1 я и сам уже думал но как освободить еще не думал. Спасибо что посмотрел мой "первый блин".Очень благодарен!

Всё тоже самое, но по-человечески:

[QUOTE=madjahed]Всё тоже самое, но по-человечески:
[/QUOTE]
М-да... вижу здесь мало человеческого...
Зачем if внутри цикла? Для снижения производительности и запутывания кода?
Кроме того автор топика возвращает строку, память под которую выделилась в данной функции, не очень красиво, но сойдет. У тебя же результат складывается в строку, которая была создана вне функции. Какие гарантии, что хватит размера памяти, выделенную кем-то другим?
Кстати, при такой ситуации (когда память под результат выделена) тебе strlen вовсе не нужен, т.к. это лишний цикл.

Кроме того автор топика прав, что второй аргумент должен быть const char*. В твоем же случае оба первых аргумента желательно сделать const char*.

[QUOTE=aks]Какая то ненужная избыточность вызовов strlen().
Размеры уже известны можно исспользовать size_s1 + size_s2 + 1
[/QUOTE]
Это спорно. Для читабельности во многих случаях strlen лучше, а компилятор в принципе потом это дело заоптимизирует.
Дело вкуса и стиля.

[QUOTE=aks]
Можно было бы конечно вместо new исспользовать realloc(s1, i), раз уж строки вида char *. Ну да ладно, это личное дело и смешивать с new не стоит.
[/QUOTE]
По большому счету, использование realloc просто сократит две строки (delete и new) до одной и всё. Но т.к. это С++, то лучше уж new и delete.

[QUOTE=aks]
NULL - это конечно 0, но всетаки логически он применяется как нулевой указатель. Тоесть - указатель равный нулю. К простым числовым/символьным переменным его применять логически некоректно. Хотя реально конечно разницы нету.
[/QUOTE]
Эта строка вовсе не нужна, т.к. далее переменная однозначно инициализируется. Но на будущее скажу, что инициализировать char лучше даже не просто 0, а '\0'.

[QUOTE=aks]
А вот тут уже ошибка. Память выделенная на которую указывал s1 не освобождается, а в s1 присваивается указатель на другую область памяти. В итоге старый указатель и его область памяти теряются.
[/QUOTE]
Это не ошибка, а memoryleak. Неприятная штука.

[QUOTE=RedHeart]Вот код функции (это просто мое решение для упражнения из книги Страуструпа):


Вопрос что можете посоветовать начинающему программисту[/QUOTE]
Назначение команды
s1 = buf;
не совсем понятна.

[QUOTE=Мerlin]Назначение команды
s1 = buf;
не совсем понятна.[/QUOTE]
Функция возвращает результат в первом аргументе, как это делают стандартные функции str, за исключением, что память выделяется внутри функции.

[QUOTE=Green]Функция возвращает результат в первом аргументе, как это делают стандартные функции str, за исключением, что память выделяется внутри функции.[/QUOTE]В данном случае это врятли. Для этого первый аргумент должен был бы иметь тип char** и
*s1 = buf;

[QUOTE=madjahed]Всё тоже самое, но по-человечески:
[/QUOTE]
Уж DWORD тут точно не человеческое и не к месту.


[QUOTE=Green]
По большому счету, использование realloc просто сократит две строки (delete и new) до одной и всё. Но т.к. это С++, то лучше уж new и delete.
[/QUOTE]
Ну вобще realloc перераспределит память и не тронит первую строку в случае увеличения. Но в C++ лучше new конечно )

[QUOTE=Green]
Эта строка вовсе не нужна, т.к. далее переменная однозначно инициализируется. Но на будущее скажу, что инициализировать char лучше даже не просто 0, а '\0'.
[/QUOTE]
Ну я и говорил, что ch даже не нужна ))
А char конечно логично инициализировать '\0', так же как указатель инициализировать NULL. Хоть реально это одно и тоже =)

[QUOTE=Green]
Это не ошибка, а memoryleak. Неприятная штука.[/QUOTE]
Ну я то знаю что такое меморилик, а начинающего прощще сказать ошибка. =)) Меморилик впринципе и есть результат логической ошибки. =)

[QUOTE=Green]М-да... вижу здесь мало человеческого...
Зачем if внутри цикла? Для снижения производительности и запутывания кода?[/QUOTE]
Может сначала бы код посмотрел? Ты что, не видишь зачем там if?

[QUOTE=Green]Кроме того автор топика возвращает строку, память под которую выделилась в данной функции, не очень красиво, но сойдет. У тебя же результат складывается в строку, которая была создана вне функции. Какие гарантии, что хватит размера памяти, выделенную кем-то другим?
Кстати, при такой ситуации (когда память под результат выделена) тебе strlen вовсе не нужен, т.к. это лишний цикл.[/QUOTE]
Я написал эту функцию за 2 минуты "по мотивам" автора. В принципе эта конкатенация реализовывается через CopyMemory().
[QUOTE=Green]
Кроме того автор топика прав, что второй аргумент должен быть const char*. В твоем же случае оба первых аргумента желательно сделать const char*.[/QUOTE]
Это на что будет так сильно влиять, объясни мне, дурачку?

[QUOTE=aks]Уж DWORD тут точно не человеческое и не к месту.[/QUOTE]
Чего так?

[QUOTE=Мerlin]В данном случае это врятли. Для этого первый аргумент должен был бы иметь тип char** и
*s1 = buf;[/QUOTE]
Да, согласен.
char** или char*&

[QUOTE=madjahed]Может сначала бы код посмотрел? Ты что, не видишь зачем там if?
[/QUOTE]
Неа не вижу... понятнее и быстрее будет так:

Почему этот код быстрее и понятнее, нужно объяснять?
А почему и как обойтись без strlen надо объяснять?

[QUOTE=madjahed]
Это на что будет так сильно влиять, объясни мне, дурачку?
[/QUOTE]
Это будет сильно влиять на то, какие строки ты сможешь складывать.
В твоем варианте нельзя складывать константные строки:

А в большинстве случаев одна из частей (обычно вторая) бывает константной.

[QUOTE=madjahed]Чего так?[/QUOTE]
Ну функция то общего назначения, без всякой привязки к чему либо. Так зачем нагромождать ее нестандартными типами.

[QUOTE=Green]Неа не вижу... понятнее и быстрее будет так:

Почему этот код быстрее и понятнее, нужно объяснять?[/quote]
Этот код быстрее и понятнее, потому что S2 затирает S1 в строке Result? :)

Меня интересовала бы, как можно определить размер выделяемой памяти, не зная длину S1, S2.
Предварительно перебрать символы S1/S2 пока не попадется '\0'? Но это тот же самый strlen().

А если catstr((char *)str1, (char *)str2, result); ?

Еще один вариант catstr()

[QUOTE=Мerlin] [/QUOTE]
Раз уж это C++, то логичней наверно будет сделать:

[QUOTE=aks]Раз уж это C++, то логичней наверно будет сделать:
[/QUOTE]Хоть это не С++, а С, но пусть будет, так как вместо catstr(&s1, s2) можно будет писать catstr(s1, s2);

[QUOTE=Мerlin]Хоть это не С++, а С, но пусть будет[/QUOTE]
Если это С, а не С++, что тогда в коде делают например new/delete =))

[QUOTE=aks]Если это С, а не С++, что тогда в коде делают например new/delete =))[/QUOTE]Где ты там видишь new/delete? :D
Под С я имел ввиду стиль программирования.
Потом new/delete еще можно заменить на malloc/free. Но на что заменить ссылку?

[QUOTE=Мerlin]Где ты там видишь new/delete? :D
[/QUOTE]
=))) Нехорошо код менять послек коментария =)

[QUOTE=Мerlin]
Под С я имел ввиду стиль программирования.
[/QUOTE]
Ну а я имелл ввиду всетаки реализацию. Раз уж исспользуеться new/delete и т.п. то вправе исспользовать и другие конструкции языка. Так что почему бы не ссылку.
А еслиб ссылки не было - то и указателем можно.
Только тогда проще все же требовать наличия памяти на обе строки, как это в реальном strcat. C way всетаки =)

По-моему, как-то более естественно выглядит, нафига посимвольно в цикле копировать? Надо думать, соответствующие функции memcpy/strcpy ассемблерные и работают быстрее, чем цикл

[QUOTE=Мerlin]Этот код быстрее и понятнее, потому что S2 затирает S1 в строке Result? :)
[/QUOTE]
Нет, ты не угадал. Я не дописал по невнимательности
Result[s1len+i] = S2;
но код быстрее не по этому.

[QUOTE=Мerlin]
Меня интересовала бы, как можно определить размер выделяемой памяти, не зная длину S1, S2.
[/QUOTE]
Никак.

[QUOTE=Мerlin]
А если catstr((char *)str1, (char *)str2, result); ?
[/QUOTE]
Ну есть и другие пути через ж..пу.
А делать const_cast - очень дурной тон (я бы по рукам бил).

[QUOTE=Мerlin]
Еще один вариант catstr()

Свернуть исходник 

Код:

char* catstr(char** s1, char* s2)
{
  unsigned size_s1 = strlen(*s1);
  unsigned size_s2 = strlen(s2);
  unsigned n = size_s1 + size_s2 + 1;
  char *buf=new char[n];
  char *cbuf = buf + n - 1;

  char *ch = s2 + size_s2;
  for(;ch>=s2;ch--)*cbuf-- = *ch;
  ch = *s1 + size_s1 - 1;
  for(;cbuf>=buf;cbuf--)*cbuf = *ch--;

  delete[] *s1;
  *s1 = buf;

  return *s1;
}

[/QUOTE]
IMHO как-то неаккуратно. Аккуратнее было бы так:

 

Код:

char* pbuf = buf;
        char* p    = *s1;
        do *pbuf++ = *p++; while(*p);
        while(*pbuf++ = *s2++);

Для случая, когда строка алоцируется из-вне вообще все компактно получается:

 

Код:

char* catstr(char* s1, const char* s2)
{
    char* p = s1;      
    while(*p) p++;
    while(*p++ = *s2++);
    return s1;
}

[QUOTE=cheburator]По-моему, как-то более естественно выглядит, нафига посимвольно в цикле копировать? Надо думать, соответствующие функции memcpy/strcpy ассемблерные и работают быстрее, чем цикл[/QUOTE]
Это вопрос академический, а не практический.
На практике можно и strcat использовать и basic_string::operator+

[QUOTE=Green]Это вопрос академический, а не практический.
На практике можно и strcat использовать и basic_string::operator+[/QUOTE]
Если вопрос академический, я думаю, подразумевается что строковыми функциями вообще нельзя пользоваться. В том числе и strlen, который почему-то использовался в большинстве постов.
Значит, немного дорабатываем и получаем что-то типа:

Ну, или, если уж на то пошло, меняем memcpy на цикл. Главное - memcpy не строковая функция.

Ну а кто сказал, что автор ещё не реализовал свой аналог strlen?
Зачем же его лепить во внутрь?

[QUOTE=Green]Ну а кто сказал, что автор ещё не реализовал свой аналог strlen?
Зачем же его лепить во внутрь?[/QUOTE]
Гы :) А кто сказал, что автор ещё не реализовал свой аналог strcpy? :) См. сообщение 22