Comments 85
… И существенно менее читаемый код. В модуль не смотрел, но если это работает в цикле и вызывается часто — быстродействие важнее, но если 1-2 раза на упаковку — это плохой, вредный код. Ну или хотя бы надо оставить старый код как комментарий, для понимания.
length := len(b)
if b == nil {
это теперь нормально — сначала использовать аргумент, а потом проверять его на пустое значение?
Собственно это и хотел исправить автор вначале =)
len(nil) в go возвращает ноль
и это сделал человек, утверждавший, что «errors are values»?
len(nil) в go возвращает ноль. Так записано для читаемости.
Сложно назвать это читаемым. Говнокод как говнокод.
Я бы немного уточнил.
Магия тут не в len() — len всегда делает одно и то-же: берет поле Len из слайса, который представляет из себя структуру вида.
type Slice struct {
Data uintptr
Len int
Cap int
}Магия тут в "касте" nil в слайс ( при сравнение или присвоении ). Сравнение слайса с nil — это сравнение со Slice{0,0,0} что является значением по умолчанию при иницализации переменной слайса. В результате — только-что созданный слайс == nil
Зачем так было сделано? Как уже было написано — возможность работать со слайсом без дополнительных проверок. Это из того-же порядка что и отсутсвие необходимости инициализировать переменную типа bool.
То есть считается, что код вида
length := len(b)
if b == nil {
// No byte slice present. Assume string s should be encoded.
length = len(s)
}менее понятный, чем
if b == nil {
length := len(s)
} else {
length := len(b)
}Кажется, я был прав когда не стал изучать этот язык...
Во втором примере вы на самом деле объявляете две разные переменные length, каждая в своей области {...}. Использовать их за пределами этой области не получится, там они не будут существовать. Такой код не скомпилится, потому что в Go нельзя объявлять локальную переменную и не использовать ее. Поэтому придется писать так:
var length int // Объявление
if b == nil {
length = len(s)
} else {
length = len(b)
}
// Используем lengthЭто значительно длинее первого варианта, хотя и понятнее.
Спасибо за исправление.
Это значительно длинее первого варианта, хотя и понятнее.
Ну так это же практически девиз языка go — "длиннее и понятнее". Но, видимо, даже у авторов стандартной библиотеки нету сил следовать ему полностью...
Это значительно длинее первого варианта, хотя и понятнее.
В любом адекватном языке:
length := (b == nil) ? len(s) : len(b)Или, даже, так:
length := len(b == nil ? s : b)А иногда и так::
length := len(b || s)И коротко, и понятно. Но в гоу все через жопу.
В то время как
len(b || s) — читается очень просто: дай длину b (если существует) или s
Можно я с вами поспорю? :) Я вот, не зная этой конструкции, прочитал "дай длину b или s… ну чего-нибудь из этих двоих, всё равно чего". Нет, серьёзно, вот этого "если существует" в этой записи нигде нет, руку на сердце положа. Это такая же особенность языка как length(nil) в Go, которую тоже надо знать, просто она для вас привычнее. Гхм, ну и length(nil) тоже в принципе логику некую имеет — чему равна длина чего-то несуществующего? Ничему, нулю. (Если что, я не пишу ни на Go, ни на JS, просто погулять вышел.)
Дело в том, что я знаю такую особенность Гоу (я на нем писал последнее время) и мне она кажется крайне нелогичной — это костыль в спецификации, потому что нету удобного инструмента.
чему равна длина чего-то несуществующего?
Ошибке (потому что в Гоу обычно возвращается ошибка в таких нелогичных случаях).
Но проблема ведь не только в len(nil), но и в том, что сперва ты присваиваешь неправильное значение, а потом проверяешь условия и присваиваешь правильное! Бред ведь.
Еще раз, я писал на Гоу, мне привычный этот синтаксис и я все-равно считаю его ущербным.
Возможно, в каком-то языке это такой специальный оператор, у которого именно такая задача. Возможно, в другом языке это вообще бы значило "дай длину true если b — true или a — true, или длину false если оба false" — и, нет, я не могу ответить, какой физический смысл у len(true). :) Хорошо, для тех, кто пишет на C#, этот код читается просто. Откуда следует, что он универсально простой для всех? Для его понимания как минимум надо знать особенность конкретного языка, вот я про что.
Про длину несуществующего — я говорю не с точки зрения соответствия вообще конвенциям языка (я так глубоко в Го не лез, не моя епархия), а просто с точки зрения логики, не привязанной к какому-то языку. У ничего и длина ничего.
Да, если что — я ни разу не защищаю код из статьи, присвоение а потом проверка (для меня, но я не пишу на Го) вызывает содрогание. Но вот для примера — если бы там дальше стоял цикл (гипотетически, в какой-нибудь другой функции) и он бы при b = nil корректно отрабатывал 0 раз (поскольку len(nil) = 0) и функция возвращала бы пустую коллекцию, то это могло бы читаться нормально. По Мартину прямо. Так что я не думаю, что len(nil) = 0 это так уж нелогично. Код из статьи — ну, да. Но если я скажу "но логичнее было бы возвращать ошибку" или "логичнее было бы выбрасывать исключение", то я просто буду примерять ко всему набор вывихов, которые у меня в мозгу для другого языка, и не факт, что они на самом деле логичнее, просто я к ним привык.
И если второе очевидно, то первое объясню — обычно (та почти всегда, наверное) Гоу не позволяет делать таких вещей и выбрасывает ошибку. Та он выбрасывает ошибку даже когда ее можно не выбрасывать, просто чтобы выбросить.
Временно закомментил код, но не закомментил импорт — ошибка «imported and not used». Блин, я же дебажу, я его потом разкоменчу! Но нет, го--- оправдывает свое название и вместо того, чтобы написать ворнинг — падает в панику
package main
import "fmt"
func main() {
// fmt.Println("Hello, playground")
}Разрабатываешь, пишешь логику, заранее объявил переменные, чтобы потом заюзать, решил проверить, как оно работает часть, которую написал, запускаешь. Фиг там — паника «declared and not used».
package main
func main() {
var arr []int = nil
}Ну то есть этот код никак не влияет на производительность, компилятор знает, что он лишний (все-равно провел анализ) и может быть спокойно выброшен, но нет ведь — ошибка
Имеем две структуру, которые циклически ссылаются одна на другую. Все работает корректно. Рефакторим, разносим их по разным неймспейсам. Все упало, внезапно гоу осознал циклическую зависимость! И я понимаю, что иногда это свидетельствует о неудачном дизайне, но я хочу сам решать. Тем более, самое популярное решение — не изменить архитектуру, а забить на рефакторинг и все свалить в одну
Я уж молчу о тошнотворном повторении как мантры
if err != nil (это настолько въедается, что ты просто раз за разом копипастишь, как в анекдоте про студента и гвозди). Раз гвоздь, два гвоздь, но у тебя уже их в заднице столько, что ты привык:a, err := doA()
if err != nil {
return nil, err
}
b, err := doB(a)
if err != nil {
return nil, err
}
c, err := doC(b)
if err != nil {
return nil, err
}Видите, как Go относится к таким ошибкам? Это и есть Go-way. И гоу-штунды кричат: «это не так плохо», «это даже клево», ведь это очевидно, ты должен контролировать свой код, ты должен писать свою архитектуру, ошибка должна быть явной, рекурсивная зависимость — плохой код, срочно рефакторите.
Но последовательность им не знакома, потому, на говнокод они кричат: «зато так покороче», ну и что, что он nil воспринимает как пустой массив, зато так удобно, нет ничего страшного, что я сначала присваиваю переменной неправильное значения, вызывая функцию len, когда ее не надо вызывать!
Зато неиспользуемые import'ы использовать нельзя — ведь Гоу следит за чистотой кода, видите ли.
… это кривой код как с точки зрения Гоу, так и с точки зрения computer science в целом.
Это про len(nil), или именно про код из статьи? Если про код — да, я в принципе не спорю, можно было создать переменную без лишнего вызова len() с потенциальным мусором в аргументе. Я только про то, что контракт len(nil) -> 0 в принципе не криминал и может пригодиться в какой-нибудь другой ситуации.
С неиспользуемыми переменными и импортами да, тоже перегиб. Согласен — иногда хочется что-то набросать и проверить, или выкусить фрагмент на время. (Вроде goimports хотя бы об импортах позволяет забыть, но это если IDE.)
А что видится альтернативой цепочкам if-ов с проверками err? Исключения? Вообще — я отчасти понимаю, почему их нет. Язык бедный, он просто намеренно бедный — ему вроде бы как раз в заслугу ставят то, что команда вчерашних студентов может через неделю выдавать более или менее удобоваримый код, а не сидеть с опухшими головами и отвлекать сеньоров вопросами. Исключения всё-таки предполагают осознание того, что у нас в коде куча неявных точек возврата, кривая обучения сразу круче становится. (Если что, я за выразительные языки. Просто философски отношусь к тому, что рынок хочет толпу годных в дело стажёров.)
Язык бедный, он просто намеренно бедный — ему вроде бы как раз в заслугу ставят то, что команда вчерашних студентов может через неделю выдавать более или менее удобоваримый код, а не сидеть с опухшими головами и отвлекать сеньоров вопросами.
Она в любом случае не может. Какие такие задачи они будут решать? Люди не спотыкаются, о синтаксис языка, если это только не какой-то haskell, люди страдают от сложности библиотек и фреймоворков, но тут то go даже хуже, чем другие языки, потому что выразительные конструкции заменяются на структуры и кучи шаблонного кода.
Я не уверен, что это корректный аргумент. Вроде есть отзывы о том, что новички очень быстро становится продуктивным на Го (на бэкенде во всяком случае). И с другой стороны, выразительные конструкции могут, наоборот, усложнить понимание фреймворков и библиотек. Регулярно слышу, как народ стонет по поводу библиотек на Scala, которые без автора сопровождать невозможно.
a, err := doA() if err != nil { return nil, err } b, err := doB(a) if err != nil { return nil, err } c, err := doC(b) if err != nil { return nil, err }
интересно, а как при этом принято логировать такие ошибки?
let length = if let Some(s) = s {s.len()} else {d.len()};let length = s.unwrap_or(d).len();
конкатенация строк гораздо быстрее, чем fmt.Sprintf().
После того как я поменял последнюю строчку на record := fmt.Sprint(size) + " " + k + "=" + v + "\n", я увидел значительное ускорение
Почему компилятор сам этого не делает?
Почему компилятор не знает всё про внутреннюю логику и побочные эффекты какой-то функции какого-то пакета и не заменяет её вызов на вызов другой функции? Ну, что Вам сказать… придётся признать, что разработчики компилятора невероятно ленивы и толком не умеют писать компиляторы.
А если серьёзно, то, как уже упоминали выше, такого рода оптимизации во-первых зачастую преждевременные, во-вторых ухудшают читабельность, в-третьих в следующих версиях языка и/или пакета fmt более медленный вариант сейчас может оказаться более быстрым, в-четвёртых если уж очень хочется, то рекомендации по такого рода оптимизациям должны давать внешние утилиты вроде статических анализаторов кода (линтеров) или IDE.
А часто ли? Я вот просто не уверен, что стоит огород городить для очень частного случая. Если у горстки человек sprintf() внезапно окажется одной из самых долгих вещей на горячем пути выполнения — ну, наверное они этот путь всё равно будут профилировать, и сделают то же, что сделал автор оригинальной статьи. А просто так, чтобы было… не знаю. Да даже подвиг автора взять — скажем, взять большущий архив, миллион файлов, по… скажем, пять записей в расширенном pax-заголовке для каждого из файлов. Двести наносекунд экономии на запись, и получится экономия аж в одну секунду на создание архива. Кошкины слёзы, а читаемость кода ухудшилась.
Лучше было бы создать функцию вычисления длины (пишу в псевдокоде):
int getLength(string b, s)
{
if (b != null)
return len(b);
return len(s);
// return b != null ? len(b) : len(s);
}А затем в основной функции вызвать ее так:
length = getLength(b, s);Что получаем:
Код основной функции становится чище — в нем показывается, что нужно делать, а не как делать.
Во вспомогательной функции последовательно проверяем "особые" условия (которые теоретически могут добавляться), каждый раз return-имся, в конце возвращаем основное значение.
Да, избавляемся от действительно не очень красивого "else" в коде основной функции.
И самое главное — избавляемся от того, что код, приведенный в начальном варианте:
length := len(b) if b == nil { // No byte slice present. Assume string s should be encoded. length = len(s) }
не просто не оптимален, но и не отражает сути того, что нужно получить в результате работы этого кода, а значит, потенциально может уже содержать ошибки, не является легко сопровождаемым и масштабируемым.
- А на возражение, что вызов функции — накладные расходы, ответить очень просто: а вот пусть в этом случае компилятор показывает чудеса оптимизации и инлайнит вызов этой функции.
Если вспомогательная функция находится в том же модуле/пакете/сборке, и не видна наружу (internal), то ее вполне можно заинлайнить по умолчанию; хотя можно и помочь компилятору, указав соответствующий атрибут, если таковой есть в языке/платформе.
Интересно, а какие доводы в пользу неудачного синтаксиса вы ожидаете? Исследований, которые скажут, что да, вот такой синтаксис приводит к 100к ошибкам в месяц?
Представляется, что в этом в т.ч. и состоит искусство разработчика — исходя из опыта и знания предметной области уметь загодя определять, будут ли разрастаться в будущем "особые" условия, потребующие отдельной функции.
И если "да" — то эту функцию лучше сделать сразу, т.к. практика показывает, что "потом", особенно при совместном владении кодом, никто не делает декомпозицию, необходимость которой появилась. В итоге разрастаются простыни кода.
Также верно, что в случае, когда функция имеет значение только в контексте какой-либо другой функции, то ее имя требует осмысления (т.к. тот же getLength представляется именем функции общего назначения, а не контекстно-зависимой).
На мой взгляд, хорошим решением здесь является механизм локальных функций или лямбд (насколько знаю, в Golang нет локальных функций, но есть лямбды).
Но вы почему-то написали несколько придирок не по существу к моим тезисам, и проигнорировали основное — что речь шла о возможных способах замены этого кода:
length := len(b)
if b == nil {
// No byte slice present. Assume string s should be encoded.
length = len(s)
}Это же никуда не годится.
Во-первых, нужно помнить, что первая строчка не упадет с исключением.
Точнее, в случае Go — что функция len имеет декларацию "func len(v Type) int", а не "func len(v Type) (int, error)".
Но этот код может читать и неподготовленный человек.
Во-вторых, это же хак — рассчитывать на то, что компилятор транслирует этот исходный код в бинарный так, как будто он написан через else.
Нечитаемо, и кроме того, поведение компилятора ведь может и измениться.
В 1933 году Петр Борисович Ганнушкин ввел термин "салонное слабоумие". Символично, но я готов дополнить его теорию. Те, кто не хочет признавать истину авторитетов должны быть такими вот салонно слабоумными изгнаны.
Звучит иронично, потому что 90% golang построено на авторитетный решениях, с которыми далеко не все согласны.
Аргументируют.
Или делают вид, что аргументируют. С одной стороны "у нас логичный и строгий язык", а с другой "len(nil) == 0". С одной стороны "исключения плохо, потому что везде приходится фигашить проверки", а по факту стало еще хуже, так как вместо типизированных исключений мы получаем переменную err, которую так же надо обрабатывать, но она менее прозрачная. Шаблонные типы не нужны, потому что это "усложняет язык", а вот кодогенерация — нет.
а вот пусть в этом случае компилятор показывает чудеса оптимизации и инлайнит вызов этой функции.Говорят, что функции «инлайнятся», но не слишком хорошо: только те, что не вызывают других функций.
По-моему, ваш первый пул-реквест нужно было принять (почему — написал выше), а вот второй как раз отклонить:
- Конкатенация строк вместо форматирования очень сильно снижает читаемость кода.
- Что касается снижения производительности в случае использования форматирования строк, то представляется, что функция, которая внутри себя форматирует строки, явно не предназначена для вызова в цикле из миллиона итераций.
А в случае однократного (или, пусть несколько десятков) вызова — как раз тот случай, когда можно пожертвовать производительностью даже в несколько раз ради читаемости.
Кроме того, если эта функция форматирует строки, вряд ли она предназначена для вызова рядом функциями, быстро вычисляющими какую-нибудь математику. Скорее всего, эта функция вызывает в контексте каких-то еще более медленных функций.
size := len(k) + len(v) + padding
size += len(strconv.Itoa(size))
record := fmt.Sprintf("%d %s=%s\n", size, k, v)а что будет, если len(k) + len(v) + padding = 99 ??
как я понял size так же учитывает и свою собственную длину в строковом представлении
это ж пипец
// formatPAXRecord formats a single PAX record, prefixing it with the
// appropriate length.
func formatPAXRecord(k, v string) string {
const padding = 3 // Extra padding for ' ', '=', and '\n'
size := len(k) + len(v) + padding
size += len(strconv.Itoa(size))
record := fmt.Sprintf("%d %s=%s\n", size, k, v)
// Final adjustment if adding size field increased the record size.
if len(record) != size {
size = len(record)
record = fmt.Sprintf("%d %s=%s\n", size, k, v)
}
return record
}
ps
формат PAX record весьма неудачен, имха
pps
CL автора такие не принят
ну
func formatPAXRecord(k, v string) (string, error) {
const padding = 3 // Extra padding for ' ', '=', and '\n'
size := len(k) + len(v) + padding
sizeLen := len(strconv.Itoa(size))
size += sizeLen
sizeStr := strconv.Itoa(size)
if len(sizeStr) != sizeLen {
sizeStr = strconv.Itoa(size + len(sizeStr) - sizeLen)
}
record := sizeStr + " " + k + "=" + v + "\n"
return record, nil
}
не знаю насколько читаем этот вариант, но для начального кода:
BenchmarkFib10-8 10000000 163 ns/op 32 B/op 2 allocs/opдля этого
BenchmarkFib10-8 20000000 94.9 ns/op 16 B/op 1 allocs/opтоже чтоли пул риквест сделать
по поводу аллока — это то что выделяется в куче, то что выделяется на стеке там не показывается (ибо быстро и так легко не отследишь)
кстати это на 1.9 были результаты ради интереса проверил на 1.8:
BenchmarkFib10-8 5000000 247 ns/op 32 B/op 5 allocs/op
BenchmarkFib10-8 10000000 181 ns/op 16 B/op 4 allocs/opполучается неплохо так доработали компилятор в 1.9 — как миниму он сумел все вызовы strconv.Itoa на стеке разместить
там целом повторяется строка
strconv.Itoa(size) + " " + k + "=" + v + "\n"как и исходная
fmt.Sprintf("%d %s=%s\n", size, k, v)
Как новичок в Go контрибьютил