Добрый день.
Давненько я не брал в руки плюсы...
Буду признателен за критику кода решения задачки http://acm.timus.ru/problem.aspx?space=1&num=1712
Волнует в первую очередь move semantics / передача и возврат объектов между функциями.
Возможно ли repeat сделать проще? ну лень мне каждый раз писать for(;) ))
#include <iostream>
#include <string>
#include <array>
#include <algorithm>
#include <utility>
#include <iterator>
static const size_t SQUARE_DIMENSION = 4;
using square_t = std::array<std::string, SQUARE_DIMENSION>;
template <typename F>
void repeat(unsigned n, F f) {
while (n--) f();
}
inline square_t read_square(std::istream &in){
square_t out;
std::generate_n( std::begin(out), SQUARE_DIMENSION, [&](){
std::string line;
getline(in, line);
return std::move( line );
});
return std::move( out );
}
inline square_t& rotate(square_t &in){
for (size_t i = 0; i < SQUARE_DIMENSION; i++) {
for (size_t j = i; j < SQUARE_DIMENSION; j++) {
std::swap( in[i][j], in[j][i] );
}
}
for(size_t i = 0; i < SQUARE_DIMENSION/2; i++) {
for(size_t j = 0; j < SQUARE_DIMENSION; j++) {
std::swap( in[j][i], in[j][SQUARE_DIMENSION - i - 1] );
}
}
return in;
}
inline std::string merge(const square_t &ciphered, const square_t &grille){
std::string out;
for (size_t i = 0; i < SQUARE_DIMENSION; i++){
for (size_t j = 0; j < SQUARE_DIMENSION; j++){
if ( grille[i][j] == 'X' )
out += ciphered[i][j];
}
}
return std::move( out );
}
int main()
{
auto grille = read_square( std::cin );
auto ciphered = read_square( std::cin );
repeat(4, [&](){
std::cout << merge( ciphered, grille );
rotate(grille);
});
return 0;
}
Здравствуйте, malegkin, Вы писали:
M>Волнует в первую очередь move semantics / передача и возврат объектов между функциями.
С этим тут плохо.
Простой return out; и короче записывается и работать будет быстрее.
А return std::move( out ); — это пессимизация.
Код вида return std::move(something) нужен совсем в других ситуациях (очень редких), например во всяких string_builder'ах:
1. Конструкции return std::move обычно вредны, потому как ломают RVO.
2. в функции rotate() цикл видимо должен начинаться так for (size_t j = i + 1 ... иначе не понятно кто с кем меняется. Вообще функция мутная, я что-то не разобрался, что она делает
3. в функции merge неплохо бы сделать out.reserve(SQUARE_DIMENSION). Также это неплохо бы сделать в функции read_square() line.reserve(SQUARE_DIMENSION) потому как функция getline вычитывает из потока посимвольно
M>template <typename F>
M>void repeat(unsigned n, F f) {
M> while (n--) f();
M>}
M>
В качестве наиболее универсального способа передачи функтора в функцию имеет смысл использовать форвардную ссылку:
template <typename F>
void repeat (usnsigned n, F&& f) {
while (n--) f();
}
Это не обычная rvalue ссылка, как может показаться на первый взгляд, а так называемая forwarding reference. Она может превратиться как в rvalue ссылку, так и в lvalue — константную, или неконстантную. Тем самым обеспечит автовыведение наиболее подходящего типа ссылки, подобранного в соответствии с типом фактического параметра, и убережет от нежелательного копирования/перемещения, что может быть актуально для "тяжелых" функциональных объектов и лямбд.
--
Не можешь достичь желаемого — пожелай достигнутого.
Здравствуйте, malegkin, Вы писали:
M>static const size_t SQUARE_DIMENSION = 4;
Может мелкая придирка, но я в своем коде стараюсь не использовать all upper case имена,
т.к. есть вероятность пересечься с каким-нибудь макросом.
M> return std::move( out );
В return std::move не нужен. Компилятор сам сделает перемещение при возврате по значению и доступности перемещения.
Для std::string и массива из них перемещение, конечно, полезно.
M>inline square_t& rotate(square_t &in){ M> ... M> return in; M>}
Странная сигнатура функции. Принимаем lvalue и его же возвращаем?
Какие-то цепочки преобразований записывать?
Для меня было бы естественней: void rotate(square_t &in)
Здравствуйте, watchmaker, Вы писали:
M>>Волнует в первую очередь move semantics / передача и возврат объектов между функциями. W>С этим тут плохо. W>Простой return out; и короче записывается и работать будет быстрее.
спасибо за пояснение. давно читал про RVO, казалось, что он требует явного std::move (
Здравствуйте, sergii.p, Вы писали:
SP>1. Конструкции return std::move обычно вредны, потому как ломают RVO.
ээх плохо и давно про RVO читал, казалось наоборот ( пошел перечитывать )
SP>2. в функции rotate() цикл видимо должен начинаться так for (size_t j = i + 1 ... иначе не понятно кто с кем меняется. Вообще функция мутная, я что-то не разобрался, что она делает
ага мутная ) тестов для нее еще не хватает , но в задачке проще было отладить через принт-дебаг ) а так она норм работает. поворачивает решетку Кардано для одноименного шифра ) в
SP>3. в функции merge неплохо бы сделать out.reserve(SQUARE_DIMENSION). Также это неплохо бы сделать в функции read_square() line.reserve(SQUARE_DIMENSION) потому как функция getline вычитывает из потока посимвольно
в общем случае для произвольного SQUARE_DIMENSION само собой. спасибо за напоминание, не подумал об этом. но для 4 и соответсвенно 16 символов в блоке, Small String Optimisation должны меня спасти ) там какраз, кажется 16 символов )
SP>в остальном мне код понравился
Здравствуйте, rg45, Вы писали:
R>Это не обычная rvalue ссылка, как может показаться на первый взгляд, а так называемая forwarding reference. Она может превратиться как в rvalue ссылку, так и в lvalue — константную, или неконстантную. Тем самым обеспечит автовыведение наиболее подходящего типа ссылки, подобранного в соответствии с типом фактического параметра, и убережет от нежелательного копирования/перемещения, что может быть актуально для "тяжелых" функциональных объектов и лямбд.
Спасибо большое
Ох намудрили же в плюсах с функторами .. rvalue/lvalue это отдельная боль для меня. пошел курить доки )
Здравствуйте, qaz77, Вы писали:
Q>Здравствуйте, malegkin, Вы писали:
M>>static const size_t SQUARE_DIMENSION = 4; Q>Может мелкая придирка, но я в своем коде стараюсь не использовать all upper case имена, Q>т.к. есть вероятность пересечься с каким-нибудь макросом.
спасибо за замечание. вы правы. но ради маленькой задачки очень лень местами заморачиваться )
M>> return std::move( out ); Q>В return std::move не нужен. Компилятор сам сделает перемещение при возврате по значению и доступности перемещения. Q>Для std::string и массива из них перемещение, конечно, полезно.
Да уж возврат из функции новых объектов , особенно с учетом того, что RVO как я понял может и не отработать, это печаль боль ((
M>>inline square_t& rotate(square_t &in){ M>> ... M>> return in; M>>} Q>Странная сигнатура функции. Принимаем lvalue и его же возвращаем? Q>Какие-то цепочки преобразований записывать? Q>Для меня было бы естественней: void rotate(square_t &in)
спасибо, что-то не обращал внимания, что алгоритмы стандартной библиотеки, также не возвращают ничего из себя. как-то со времен перегрузки операторов разных осталась привычка возвращать объект назад ) что в данном случае излишне. использование rotate(rotate(rotate(it))) это ахтунг какой-то )
Здравствуйте, malegkin, Вы писали:
M>Давненько я не брал в руки плюсы... M>Буду признателен за критику кода решения задачки http://acm.timus.ru/problem.aspx?space=1&num=1712 M>Волнует в первую очередь move semantics / передача и возврат объектов между функциями. M>Возможно ли repeat сделать проще? ну лень мне каждый раз писать for(;) ))
Мои 5 копеек — inline не нужен практически всегда. Единственное его применение — определение функции в заголовочных файлах, чтобы предотвратить ошибки линкора о множественных именах для такой функции (а также inline variable начиная с C++17).
Еще вместо std::string для строк square_t можно сделать двумерный массив символов, но это вопрос вкуса.
И repeat можно сделать проще, написав обычный цикл for, знакомый 100% читателей
Здравствуйте, malegkin, Вы писали:
M>Добрый день. M>Давненько я не брал в руки плюсы... M>Буду признателен за критику кода решения задачки http://acm.timus.ru/problem.aspx?space=1&num=1712 M>Волнует в первую очередь move semantics / передача и возврат объектов между функциями. M>Возможно ли repeat сделать проще? ну лень мне каждый раз писать for(;) ))
Можно вообще без плюсов сделать
#include <stdio.h>
#include <stdlib.h>
enum { N=4, NN=N*N };
int rot(int c) { return N*(c%N)+N-1-c/N; }
int cmp(const void* a, const void* b){ return *(int*)a - *(int*)b; }
void sort(int *x,int n) { qsort(x,n,sizeof(*x),cmp); }
void read(char* s) {
int x,y,i,j; enum { line_max=80 }; char line[line_max];
for(i=0,y=0;y<N;y++) {
fgets(line,line_max,stdin);
for(j=0,x=0;x<N;x++) {
s[i++]=line[j];
if (line[j]) j++;
}
}
}
int main(int argc,char** argv) {
int i,j, nxy, xy[NN]; char s[NN];
read(s);
nxy=0; for(i=0;i<NN;i++) if (s[i]=='X') xy[nxy++]=i;
read(s);
for(i=0;i<4;i++) {
for(j=0;j<nxy;j++) {
putc(s[xy[j]],stdout);
xy[j]=rot(xy[j]);
}
sort(xy,nxy);
}
putc('\n',stdout);
return 0;
}
Здравствуйте, PM, Вы писали:
PM>Мои 5 копеек — inline не нужен практически всегда. Единственное его применение — определение функции в заголовочных файлах, чтобы предотвратить ошибки линкора о множественных именах для такой функции (а также inline variable начиная с C++17).
о про inline variable не знал, спасибо. Хотя у меня C++14 пока в проработке )
PM>И repeat можно сделать проще, написав обычный цикл for, знакомый 100% читателей
вся прелесть рипита, на мой взгляд, какраз в том, чтобы не писать постоянно for(;){} для банального повторение 10 раз какого-то тупого действия. Понятно, что в боевом коде, это избыточно, но во всяких задачках, постоянно встречается необходимость повторить N раз какое-то тупое действие, причем по нескольку раз за задачу ) имхо, жалко, что сам язык не позволяет делать фишки типа:
Здравствуйте, sergii.p, Вы писали:
SP>Здравствуйте, malegkin, Вы писали:
SP>1. Конструкции return std::move обычно вредны, потому как ломают RVO.
А на RVO есть хоть какая-то гарантия? Вроде же там как левая пятка компилятора захочет
Если же нужна гарантия, то ничего лучше указателей вроде еще не придумали. Или нет?
S>А на RVO есть хоть какая-то гарантия? Вроде же там как левая пятка компилятора захочет S>Если же нужна гарантия, то ничего лучше указателей вроде еще не придумали. Или нет?
Guaranteed copy elision — в C++17 компилятор не имеет право копировать/перемещать объект. Даже если полностью выключить оптимизацию, и даже если его попробовать явно об этом попросить. Например, в gcc или clang можно передать ключ -fno-elide-constructors и сравнить генерируемый код для C++14 и C++17 — во втором случае return Object{...}; не будет копироваться или перемещаться.
(это именно для RVO, для NVRO чуть другие правила, да).
Upd:
На всякий случай уточню, это guaranteed copy elision введено не для оптимизации (это побочный эффект), а для того, чтобы можно было на С++ фабрики нормально писать и возвращать из функций экземпляры классов, у которых нет конструкторов копирования или перемещения:
class Obj {
private:
Obj(int);
public:
Obj(Obj&&) = delete;
Obj(const Obj&) = delete;
};
Obj CreateObj() {
return Obj(4); // нельзя до C++17
}
Здравствуйте, qaz77, Вы писали:
Q>Здравствуйте, malegkin, Вы писали:
M>>static const size_t SQUARE_DIMENSION = 4; Q>Может мелкая придирка, но я в своем коде стараюсь не использовать all upper case имена, Q>т.к. есть вероятность пересечься с каким-нибудь макросом.
Всю жизнь использовал и не было проблем с пересечением.
Вменяемые писатели макросов всегда используют какой-нибудь уникальный префикс как BOOST_PP_*.
Здравствуйте, _NN_, Вы писали:
_NN>Всю жизнь использовал и не было проблем с пересечением.
А вот в моей практике были, правда давно, когда студия макросы в коде еще не умела подсвечивать.
_NN>Вменяемые писатели макросов всегда используют какой-нибудь уникальный префикс как BOOST_PP_*.
Вменяемые — да. Как позитивный пример могу привести CURL.
Проблемы начинаются после #include <windows.h>, где куча макросов без всяких префиксов.
Но, справедливости ради, там также куча макросов с буквами в разных регистрах (всякие A/W функции, например) и
min/max — классика жанра.
Здравствуйте, qaz77, Вы писали:
Q>Здравствуйте, _NN_, Вы писали:
_NN>>Всю жизнь использовал и не было проблем с пересечением. Q>А вот в моей практике были, правда давно, когда студия макросы в коде еще не умела подсвечивать.
_NN>>Вменяемые писатели макросов всегда используют какой-нибудь уникальный префикс как BOOST_PP_*. Q>Вменяемые — да. Как позитивный пример могу привести CURL. Q>Проблемы начинаются после #include <windows.h>, где куча макросов без всяких префиксов. Q>Но, справедливости ради, там также куча макросов с буквами в разных регистрах (всякие A/W функции, например) и Q>min/max — классика жанра.
Работать с windows.h без NOMINMAX ещё 20 лет назад было совсем не модно
А так да, там много чего определенно, но не припомню когда в последний раз были коллизии.
Ещё полезно добавлять весь набор NO**: NOGDICAPMASKS, NOVIRTUALKEYCODES, ... и WIN32_LEAN_AND_MEAN.
Так список возможных пересечений уменьшиться да и собираться быстрее будет.