LINUX.ORG.RU
Форум — Development  

Что я пишу неправильно в функции MoveStrings?

 ,


0

1

Задача такова: Напишите функцию MoveStrings, которая принимает два вектора строк, source и destination, и дописывает все строки из первого вектора в конец второго. После выполнения функции вектор source должен оказаться пустым.

#include <iostream>
#include <vector>
#include <string>

void MoveStrings(std::vector<std::string>& s, std::vector<std::string>& s1)
{
  for(unsigned int i = 0; i <= s.size(); ++i)
    {
      s1[i+s1.size()] = s[i];
    }
  s.clear();
}
int main()
{
  std::vector<std::string> source = {"a", "b", "c" };
  std::vector<std::string> dest = {"z"};
  MoveStrings(source, dest);
  for(auto x : source)
    std::cout << x << source.size() << "\n";
  for(auto x : dest)
    std::cout << x << "\n";

}
← Qt: компоновка формы фиксированного размера
Архитектура Android приложений. Серебряная пуля или хипстерский BS? →

Память кто за тебя выделять будет? И освой итераторы, полезно

algamest
()

s1[i+s1.size()] = s[i];

Вектор сам не расширяется автоматически. Нужно делать s1.push_back(s[i]). Или по с++11: s1.emplace_back(std::move(s[i]).

KennyMinigun ★★★★★
()
Последнее исправление: KennyMinigun (всего исправлений: 1)
Ответ на: комментарий от KennyMinigun

я так тоже делал в терминале пишет: terminate called after throwing an instance of 'std::logic_error' what(): basic_string::_M_construct null not valid Для закрытия данного окна нажмите <ВВОД>...

SerjVec
() автор топика

Перво наперво, как заслуженный хеллоуволдщик на с++ укажу на

for(unsigned int i = 0; i <= s.size(); ++i)

i <= s.size() это ошибка
unsigned int лучше заменить на decltype(s.size())

s1[i+s1.size()] = s;

Надо заменить на s1.push_back(s[i ]) или на итераторы. Если нужен именно move, то тут кто-нить из знатоков с++ ответит, я не осилил.
И

for(auto x : source)

я бы сделал auto& x

flyshoot ★
()
Ответ на: комментарий от flyshoot

шо ты от меня хочешь? я сдал всё, просто занимаюсь

SerjVec
() автор топика

Кстати по правилам хорошего тона и линейного программирования, функция не имеет права изменять значения входных элементов. Стоит принять входные данные как константы, скопировать их в третий вектор и вернуть его. Дальше уже проблема программиста использующего эту функцию, что он будет делать с исходными данными. но если пренебречь - то так: 

 
--for(unsigned int i = 0; i <= s.size(); ++i)
++for (auto it=s.cbegin();it!=s.cend();++i) 
-- s1[i+s1.size()] = s[i];
++ s1.push_back(*it);

Silerus ★★★★
()
Ответ на: комментарий от vzzo

Почему, это вполне логично. Если данные будут использоваться в расчетах не единожды - то хорошо ли, когда одна из функций будет менять входные значения? Если посмотреть в algorithm - то там существуют по два варианта функций делающих одно и тоже, но одни меняют значения входных данных, а другие нет.

Silerus ★★★★
()
Ответ на: комментарий от Stil

если мы урезаны по памяти и процессорному времени - это уже совсем другая песня, вам не кажется? Если нашу функцию используем только мы и никто более, и негде более - и она написана конкретно, для конкретного случая, то тут, не вопрос, мы вольны писать так, что бы эта функция работала так как нам хочется. Иначе придется все таки продумывать ее универсальность.

Silerus ★★★★
()
Ответ на: комментарий от dib2

не прокатит как акроним string1 string2. Хотя согласен лучше так не делать

Silerus ★★★★
()
Ответ на: комментарий от anonymous

пересдачу по русскому

Це був би дійсно найгірший кошмар для мене...

KennyMinigun ★★★★★
()
Последнее исправление: KennyMinigun (всего исправлений: 1) 
#include <iostream>
#include <vector>
#include <string>
#include <utility> // std::move

void MoveStrings(
	std::vector<std::string> &dst,
	std::vector<std::string> &src
) {
	for (auto &item : src) {
		dst.emplace_back(std::move(item));
	}

	src.clear();
}

int main() {
	std::vector<std::string>
		dst = {"a"},
		src = {"b", "c"};

	MoveStrings(dst, src);

	std::cout << "dst:\n";
	for (auto &item : dst)
		std::cout << item << '\n';

	std::cout << "src:\n";
	for (auto &item : src)
		std::cout << item << '\n';
}

💢 ./a.out
dst:
a
b
c
src:
Deleted
()
Последнее исправление: romeo250501 (всего исправлений: 1) 
#include <iostream>
#include <vector>
#include <string>
#include <utility> // std::move

void MoveStrings(
	std::vector<std::string> &dst,
	std::vector<std::string> &src
) {
    dst.insert(dst.end(), 
         std::make_move_iterator(src.begin()), 
         std::make_move_iterator(src.end()));
    src.clear();

}

int main() {
	std::vector<std::string>
		dst = {"a"},
		src = {"b", "c"};

	MoveStrings(dst, src);

	std::cout << "dst:\n";
	for (auto &item : dst)
		std::cout << item << '\n';

	std::cout << "src:\n";
	for (auto &item : src)
		std::cout << item << '\n';
}
fsb4000 ★★★★★
()
Ответ на: комментарий от Deleted

Можно std::move из алгоритмов использовать и что будет если так написать: 

 for (auto &item : src) {
		dst.emplace_back(item);
	}

SerjVec
() автор топика

Господи, всё, ну просто всё неправильно.

  • не unsigned int, а size_t, а вообще use iterators
  • ты копируешь не s.size(), а s.size() + 1 элементов
  • ты не изменил размер target вектора и пишешь не известно куда, корёжа кучу
  • ты копируешь, а не перемещаешь строки.

Для начала научись считать, освой итераторы и базовую работу с вектором (push_back). А потом std::move и константные ссылки (const auto& x : source для печати векторов в конце, чтобы не копировать строку на каждый итерации)

slovazap ★★★★★
()
Ответ на: комментарий от SerjVec

Тогда будет копирование вместо перемещения.

Deleted
()
Ответ на: комментарий от Silerus

Кстати по правилам хорошего тона и линейного программирования, функция не имеет права изменять значения входных элементов. Стоит принять входные данные как константы, скопировать их в третий вектор и вернуть его.

Чушь это собачья, как и «должна быть только одна точка выхода из функции». Ведёт только к отвратительному, нечитаемому и неэффективному коду. Правило хорошего тона только одно - аргументы которые функция модифицирует должны быть явно обозначены как таковые. В нормальном коде для этого достаточно отсутствия const у ссылки, но так как тс про const не знает, пусть хотя бы out_ приписывает. А ещё лучше, функция должна принимать не ссылки на вектора, а итераторы. И скажу больше, такая функция уже есть в стандартной библиотеке.

slovazap ★★★★★
()
Ответ на: комментарий от slovazap

Ну, наверное, хотя есть мнение, что лучше писать что-то вроде container<T>::size_type разница будет, вроде, только в шаблонном коде, но лучше привыкать заранее, верно?)

flyshoot ★
()
Ответ на: комментарий от fsb4000

dst.insert(dst.end(), std::make_move_iterator(src.begin()), std::make_move_iterator(src.end()));

dst.insert(dst.end(), std::move_iterator(src.begin()), std::move_iterator(src.end()));

Семнадцатый год на дворе, в конце-концов.

Esper ★
()
Ответ на: комментарий от Esper

Да, если C++17, то так лучше.

Кстати, раз зашел в эту тему ещё раз, то напишу во всех вариантах с for или std::move(который алгоритм) нужно обязательно перед этими for или std::move добавить dst.reserve(dst.size() + src.size());

Чтобы необходимая память сразу зарезервировалась, а не было несколько выделений памяти.

fsb4000 ★★★★★
()
Вы не можете добавлять комментарии в эту тему. Тема перемещена в архив.
← Qt: компоновка формы фиксированного размера
Development
Архитектура Android приложений. Серебряная пуля или хипстерский BS? →