Re: Это сеньорский код?
От: sr_dev  
Дата: 21.12.21 11:48
Оценка: -1
Здравствуйте, StandAlone, Вы писали:

SA>Код достался мне на поддержку. Автор доступен, но по поводу своего творения не может сказать ни слова, кроме общих предположений.

SA>Я попытался выяснить, как такое вообще попало в мастер, в ответ услышал — "Этот человек выше чем сеньор, в подробном ревью не нуждается".

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

касательно "в подобном ревью не нуждается" — понимайте это как "у человека есть дела поинтереснее чем доказывать таким как вы что-то про эту одну строчку".

кстати говоря, объявить себя умником и охотником на идиотов — это лучший способ привлечь внимание социума. я не против, привлекайте, привлекайте — но хотя бы делайте это прогрессивно что ли, более смело что ли. проще простого доковыряться до отсутсвия проверки на null, или до выражения шириной, о боже мой, в полэкрана. это же каким тупым надо быть чтобы написать выражение в полэкрана ха ха. гораздо сложнее объяснить понятным образом общественности (среди которой будут такие юмористы как вы) про то же самое отсутствие семанитики и хреновое ридабилити в масштабах проекта.
Re: Это сеньорский код?
От: Ватакуси Россия  
Дата: 21.12.21 12:25
Оценка:
Здравствуйте, StandAlone, Вы писали:

SA>Или это только во фронтенде?

Кто там работал в цирке не смеётся, да.

  public login({ username, password }: Authenticate): Promise<any> {
    username = username.toLowerCase();
    return new Promise((resolve, reject) => {
      return this.cognitoService
        .authenticate(username, password)
        .then(result => {
          let foundTenant = false;
          const tenants: Array<Tenant> = [];
          if (result.idToken.payload['custom:PlatformTenantId']) {
            const tenantDTO = JSON.parse(result.idToken.payload['custom:PlatformTenantId']);

            const tenantArray = tenantDTO.t;
            tenantArray.map((_, index) => {
              tenants.push({ id: tenantArray[index].id, name: '-' });
              foundTenant = true;
            });
          }

          const userName = result.idToken.payload.name ? result.idToken.payload.name : result.idToken.payload.email;
          const userEmail = result.idToken.payload.email;
          const userPhone = result.idToken.payload.phone_number ? result.idToken.payload.phone_number : '-';
          const exp = result.accessToken.payload.exp;

          const user: User = {
            name: userName,
            email: userEmail,
            phone: userPhone,
            tenants,
            selectedTenant: tenants[0],
            accessToken: result.getAccessToken().getJwtToken(),
            accessTokenExpiryTime: exp,
            userProfile: null
          };

          if (foundTenant) {
            return this.userService.userProfile(user).subscribe(
              up => {
                user.userProfile = up;
                // set the user in the local storage
                userUtil.set(user);
                // set the user in the service
                this.setUser(user);
                resolve(user);
              },
              () => {
                resolve(user);
              }
            );
          } else {
            resolve(user);
          }
        })
        .catch(err => {
          reject(err);
        });
    });
  }
Все будет Украина!
Отредактировано 21.12.2021 12:26 Ватакуси . Предыдущая версия .
Re[2]: Это сеньорский код?
От: Умака Кумакаки Ниоткуда  
Дата: 21.12.21 15:20
Оценка: +2 -1
Здравствуйте, Ватакуси, Вы писали:

а с этим кодом что не так? ну т.е. он абсолютно читаем, да, написан слегка небрежно, да, можно (нужно) его разбить на пару подфункций, использовать async/await вместо явных промисов, но в треде что-то похоже компания совсем непуганых собралась. Вот когда вам реально достанется код, а-ля inverse sqrt или генерённый код с кучей goto и ручными правками в нём, тогда у вас волосы на жёппе зашевелятся, а так, ну ужас, ну ужас-ужас, но не ужас-ужас-ужас же.
нормально делай — нормально будет
Re[3]: Это сеньорский код?
От: Умака Кумакаки Ниоткуда  
Дата: 21.12.21 15:37
Оценка: +1
Здравствуйте, CreatorCray, Вы писали:


CC>Но вот для куска, который привёл ТС, было бы неплохо хотя бы написать комментарий WTF там и зачем.


что блэт там понимать то? это фактически один вызов функции piecewise, которая возвращает функцию, которая маппит значения из одного диапазона на другой диапазон (это и называется scale, и у топикстартера это описано в первом посте). Я тут один самый умный или что? Или никто вообще не пытался дальше одной строки код прочесть?
нормально делай — нормально будет
Re[3]: Это сеньорский код?
От: Ватакуси Россия  
Дата: 21.12.21 19:17
Оценка:
УК>а с этим кодом что не так? ну т.е. он абсолютно читаем, да, написан слегка небрежно, да, можно (нужно) его разбить на пару подфункций, использовать async/await вместо явных промисов, но в треде что-то похоже компания совсем непуганых собралась. Вот когда вам реально достанется код, а-ля inverse sqrt или генерённый код с кучей goto и ручными правками в нём, тогда у вас волосы на жёппе зашевелятся, а так, ну ужас, ну ужас-ужас, но не ужас-ужас-ужас же.

1) Рукопашные промисы
2) Вытаскивание и втаскивание инфы в local storage — кеш себе нашли!
3) Это повторяется десятки раз по всему коду
4) Обработка ошибок очень наивная
5) Постоянные разбивки и поиски по строкам
Все будет Украина!
Re[4]: Это сеньорский код?
От: CreatorCray  
Дата: 21.12.21 19:21
Оценка:
Здравствуйте, Умака Кумакаки, Вы писали:

УК>что блэт там понимать то? это фактически один вызов функции piecewise, которая возвращает функцию, которая маппит значения из одного диапазона на другой диапазон (это и называется scale, и у топикстартера это описано в первом посте).


Напомню:
clamp(+transform.invert((input || (input = piecewise(range, domain.map(transform), interpolateNumber)))(y)));


По сути там вот что:
// pseudocode
if (!input)
    input = piecewise (range, domain.map (transform), interpolateNumber);

return clamp (+transform.invert (input, y))

Просто налеплено в одну строку непонятно зачем.

УК> Я тут один самый умный или что?

Нет, поди просто привык читать lisp

УК> Или никто вообще не пытался дальше одной строки код прочесть?

Его чтоб прочесть надо либо сначала переформатировать либо читать в редакторе с подсветкой пар скобок.
... << RSDN@Home 1.3.110 alpha 5 rev. 62>>
Re[3]: Это сеньорский код?
От: CreatorCray  
Дата: 21.12.21 19:21
Оценка: +2
Здравствуйте, Pavel Dvorkin, Вы писали:

PD>Совершенно согласен. Просто этот код не для тех, кто привык писать бизнес-приложения, и не им о нем судить. Там другие правила и требования.


Какие другие правила и требования? Писать чистый код?
Какие такие особые требования заставляют тут лепить кучу кода в одну строку?
... << RSDN@Home 1.3.110 alpha 5 rev. 62>>
Re[2]: Это сеньорский код?
От: CreatorCray  
Дата: 21.12.21 19:21
Оценка: +1
Здравствуйте, 4058, Вы писали:

4>Если честно, лень разбираться в кишках D3, но есть предположение, что автор занимается своего рода "битовыжиманием" на javascript

Да вот что то приведённый код совсем не похож на битовыжимание. Скорее на экономию байтиков файла сурсов.
... << RSDN@Home 1.3.110 alpha 5 rev. 62>>
Re[4]: Это сеньорский код?
От: ononim  
Дата: 21.12.21 19:34
Оценка:
CC>Какие такие особые требования заставляют тут лепить кучу кода в одну строку?
Может чувак код писал на смартфоне, не всеж только порно на нем смотреть.
Как много веселых ребят, и все делают велосипед...
Re[2]: Это сеньорский код?
От: StandAlone  
Дата: 21.12.21 23:46
Оценка: 5 (1) +1
Здравствуйте, DiPaolo, Вы писали:

DP>Именно этот код — да, крутого сеньора или даже выше. Для этого надо пойти посмотреть на весь файл целиком, а также понять контекст проекта. А суть там такая: это специальная узкоспециализированная либа-числодробилка (с поправкой на JS).


Нет, Вы серьезно называете аффинное преобразование в целочисленной Римановской геометрии "узкоспециализированной числодробилкой"?

DP>Тут надо вот что отметить:

DP>- код всего файла выполнен в одном стиле; аккуратный, небольшой по размеру; есть комментарии

Действительно, целых две строки комментариев.

Насчет единого стиля и содержания, марксизма и диалектики аккуратности. Знаю, всем лень ходить по линкам, поэтому вот, братишки, я вам покушать принес.
function normalize(a, b) {
  return (b -= (a = +a))
      ? function(x) { return (x - a) / b; }
      : constant(b);
}

function bimap(domain, range, interpolate) {
  var d0 = domain[0], d1 = domain[1], r0 = range[0], r1 = range[1];
  if (d1 < d0) d0 = normalize(d1, d0), r0 = interpolate(r1, r0);
  else d0 = normalize(d0, d1), r0 = interpolate(r0, r1);
  return function(x) { return r0(d0(x)); };
}


DP>- при этом важно: код изменялся буквально 4 раза за 5 лет (https://github.com/d3/d3-scale/commits/9fe496e6d708bf30435c15f03ab9e5ae994ea7be/src/continuous.js); то еть это как раз код из разряда "кака-то базовая хрень, которую написал один раз и крайне редко туда лезешь"


Правда, на этом модуле висит 19 открытых ишшуев и более 70 уже закрытых аффтаром с комментарием "пошлина, на текущем уровне развития науки невозможно" https://github.com/d3/d3-scale/issues
А так-то да,аккуратный, небольшой. Правда, можно же было вообще в одну строку написать все?
Это же и так, и сяк WRITE-only code. Ну, чукча же писатель, а не читатель... если Вы понимаете, о чем я.

DP>- ко всем проекту есть развернутое ридми с комментариями, туториалами, примерами.


Правда, если кому-то зачем-то нужно что-то на байт в сторону от ридми, то нихера не работает, а значит оно ему не надо. Сеньор же писал, сеньору виднее.

DP>Крайне все понятно. НЕ каждый сеньор сможет такое оформить. К вопросу о сеньорстве

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

А еще он внезапно former @nyt. Как Вы полагаете, не связано ли это с тем, что "фронтендеры встречались с этой либой"? Против своего желания — поскольку некое узколобое существо прочло — "ООО, оно используется в САМОЙ NY TIMES!!!111адынадын. Берем немедля"? При этом у 99.999% этих ... не хватило квалификации даже поверхностно оценить качество кода внутри?

DP>Вполне аккуратный хороший код. Да, человеку, который впервые его видит и ваще не в теме, будет ничего непонятно. Но ему и не править этот код. А при необходимости — разберется.


Да-да. Для непрофессиональных разработчиков, всяких там математиков и прочих — сойдет.

DP>Более того, бывает вообще вот такой код:

DP>[asm]

Вот-вот. Этот самый D3 старательно превращен в некое подобие ASM. Без единой на то причины, поскольку примененные подходы серьезно ухудшили перфоманс, а не улучшили. Впрочем, чтобы понять мои слова, надо хотя бы не знать JS... Поэтому не старайтесь, все уже очевидно.
Re[4]: Это сеньорский код?
От: StandAlone  
Дата: 21.12.21 23:47
Оценка: 5 (1)
Здравствуйте, bnk, Вы писали:

bnk>Чувак сделал работу за компилятор.

Какую? С моей точки зрения, он только усложнил работу интерпретатору.

bnk>Впрочем, вся D3 в стиле юниксовых команд 80-х, с дурацкими сокращениями типа x.classed() ака "class edit".


Я про нейминг, в котором clamp() это на самом деле boundaryLimits(), даже не заикаюсь. Это здесь наименьшее из прочих зол.
Re[4]: Это сеньорский код?
От: StandAlone  
Дата: 21.12.21 23:52
Оценка:
Здравствуйте, Умака Кумакаки, Вы писали:

УК> Я тут один самый умный или что?


Бугага. А Вы точно понимаете, что именно там написано?
Лично я на основании одной этой строки в теле каждой функции могу устроить беседу часа на 2-3 любому самоуверенному "знатоку JS".
По результатам которой, при желании, засыплется любой.
Re[5]: Это сеньорский код?
От: StandAlone  
Дата: 21.12.21 23:59
Оценка: 14 (2) +1
Здравствуйте, CreatorCray, Вы писали:

CC>return clamp (+transform.invert (input, y))

CC>[/code]

Почти, но нет.
clamp (+transform.invert (input ( y))

input — это локальная переменная, хранящая в себе указатель на анонимную функцию в биндинг контексте функции-хоста. Это реализация хранения состояния сущности максимально ублюдочным и криворуким способом из всех, допустимых JS, через анонимные замыкания, с кучей возможных побочных эффектов и полной невозможностью отладки.
За + же надо долго и старательно бить ногами. Когда тут пишут про "попытки оптимизации" через +, безусловно создающий новый объект типа number независимо от необходимости в этом, и без всякой обработки ошибок — мне становится плохо от смеха.

CC>Его чтоб прочесть надо либо сначала переформатировать либо читать в редакторе с подсветкой пар скобок.


Для отладки этого и аналогичного говна его, натурально, приходится сперва переписывать. А потом бороться с желанием ткнуть аффтора мордой в результат... .
Re[2]: Это сеньорский код?
От: StandAlone  
Дата: 22.12.21 00:05
Оценка:
Здравствуйте, kaa.python, Вы писали:

KP>Почему ты сам до сих пор там?


Я жертва цепи несчастных случайностей.
Как и все мы.

(с)

KP>Нет, это не нормальная ситуация. Нормальная ситуация это когда никто не может залить изменения в мастер, пока код не будет одобрен 2-3 другими разработчиками.


Так ведь ревью там было. "Покликали, работает, господин Сеньор!". И все на этом. В код никто не вкуривал, ибо не осилили, или лень.
Вот мне интересно, прочие сферы разработки ПО тоже поражены этим недугом?

KP>Мне кажется, это часть проблемы. Фронтэнд штука довольно специфичная тем, что за ней не стоит каких-то базовых знаний (теория баз данных, теория криптографии, теория распределенных систем, там даже теория алгоритмов особо и не нужна) и каждый год очередной "прорыв" который всем срочно надо внедрить.


Но ведь тут суть даже не в алгоритмах, коих нет. А общем подходе, см. имена переменных a, b в примере выше. Совершенно все базовые нормы этики и морали разработки похерены. Отсюда гармонично следует полное незнание ничего про архитектуру, паттерны, инженерию и промышленный дизайн.
Как эти люди попадают вместо условных Яндекс.Еду и Яндекс.Такси в Яндекс.Разработку?
И специфично ли это явление для РУ-области, или поражение глобально?
Re[6]: Это сеньорский код?
От: CreatorCray  
Дата: 22.12.21 01:01
Оценка: +1
Здравствуйте, StandAlone, Вы писали:

CC>>return clamp (+transform.invert (input, y))


SA>Почти, но нет.

SA>
SA>clamp (+transform.invert (input ( y))
SA>


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

Касательно остального полагаю что тебе виднее, ибо я js практически не пользуюсь (максимум для наколенной скриптоты в greasemonkey) а потому прочих тонких нюансов не знаю.
... << RSDN@Home 1.3.110 alpha 5 rev. 62>>
Подытожим
От: CreatorCray  
Дата: 22.12.21 01:09
Оценка: +1 :)))
https://coub.com/view/2wpfed
... << RSDN@Home 1.3.110 alpha 5 rev. 62>>
Re[5]: Это сеньорский код?
От: Mystic Artifact  
Дата: 22.12.21 01:45
Оценка:
Здравствуйте, StandAlone, Вы писали:

SA>Какую? С моей точки зрения, он только усложнил работу интерпретатору.

Тут жирным выделять совершенно лишнее, если не хотите подчеркнуть свою неосведомленность.

Один факт того, что JS позволяет делать forward-references — делает его не так что бы особо интерпретируемым.

А если взять и открыть V8, то под капотом там есть бэкэнд на платформе которого, сделано аж 3 JIT-компилятора. Iginition, который как бы называется интерпретатором, на самом деле генерирует байт код. И всё это в тандеме.
Re[5]: Это сеньорский код?
От: Mystic Artifact  
Дата: 22.12.21 01:58
Оценка: +2
Здравствуйте, StandAlone, Вы писали:

SA>Я про нейминг, в котором clamp() это на самом деле boundaryLimits(), даже не заикаюсь. Это здесь наименьшее из прочих зол.

Это уже похоже на придирку. clamp есть практически везде исторически. Оно уже даже есть в CSS... А формулировка boundaryLimits() это что-то очень нестандартное. Хотя, вызов clamp без задания границ конечно вызывает вопросы.
Re[4]: Это сеньорский код?
От: Pavel Dvorkin Россия  
Дата: 22.12.21 02:32
Оценка: 5 (1)
Здравствуйте, CreatorCray, Вы писали:

CC>Какие другие правила и требования? Писать чистый код?

CC>Какие такие особые требования заставляют тут лепить кучу кода в одну строку?

Лепить в одну строку, может, и не стоило бы, хотя ИМХО от разбития этой строки на части мало что бы изменилось.
Приоритеты тут другие.
Возьми любую реализацию серьезного математического алгоритма. Код, конечно, понятный, если это хорошая реализация, но обеспечить понятность кода — это не самая главная задача для автора.

Мне как-то довелось участвовать в одном проекте, и при постановке задачи я просто не знал, что делать. Речь шла о весьма специфической очистке картинки. Какой именно алгоритм тут окажется успешным — я не знал и знать не мог. Надо было пробовать, чем я и занимался. Когда, наконец, все это более или менее заработало, код я, конечно, слегка причесал перед тем, как отдать заказчику, но мне и в голову не приходило (и заказчик не требовал) переписывать его в соответствии с канонами чистого кода.
With best regards
Pavel Dvorkin
Re[5]: Это сеньорский код?
От: Умака Кумакаки Ниоткуда  
Дата: 22.12.21 04:07
Оценка: +1
Здравствуйте, StandAlone, Вы писали:

SA>Я про нейминг, в котором clamp() это на самом деле boundaryLimits(), даже не заикаюсь. Это здесь наименьшее из прочих зол.


с этого надо было и начинать. clamp это настолько типичный нейминг для этой мат.логики, что сразу выдаёт нулевой опыт у вас к компьютерной графике. отсюда и все проблемы.
нормально делай — нормально будет
Подождите ...
Wait...
Пока на собственное сообщение не было ответов, его можно удалить.