forked from agorlov/javascript-blowfish
-
Notifications
You must be signed in to change notification settings - Fork 0
/
Copy pathcode-review.txt
35 lines (29 loc) · 2.53 KB
/
code-review.txt
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
Padding method changed 2015-03-07 (@lucnap)
Coffee script not changed, only Blowfish.js and jquery.blowfish.js (do not know coffeescript)
+ pad with 0x80 followed by 0x00 bytes (OneAndZeroes Padding) See http://www.di-mgt.com.au/cryptopad.html
- trimZeros function not more needed but not removed because useful in backward compatibility with
old encrypted data
Форматирование
+ большинство кода соответсвует
- лучше использовать упрощенные конструкции условий с выходом (см. encrypt и decrypt)
Документирование
+ описания полные
- документирование должно соответсвовать http://en.wikipedia.org/wiki/JSDoc
- к методу decipher не было доков
Перменные и объявления функций
- new Array (ровно как и new Object лучше не использовать, вместо них [] и {})
- array.join(delimetr) может коряво работать в разных браузерах (не буду показывать пальцем)
при пустых элементах массива, лучше явно его заполнять
- в конструкторе не обязательно return this;
- объявление переменных через var очень внимательно:
var c = c1 = c2 = 0;// - не объявит c1 и c2, а создаст их в глобальном окружении
// надо так
var c, c1, c2;
c = c1 = c2 = 0;
- объявление переменных к классам (а так же JSDoc'и к ним) необходимо вынести из конструктора и привязать
к прототипу (там же можно инициализировать простые типы: числа, true/false, а так же строки);
причина - не придется при вызове конструктора повторно инициализировать данные
Javascript and Coffeescript
- желательно для строк использовать ", а ' использовать в иных случаях (типа 'это "строка"') и объявляя символы
в js нет такого типа как char, но можно визуально отличить его (ch = 'a')
- в js нет синтаксиса [var1, var2] = /*array*/ someFunctionReturnsArray(), а в coffee есть