Last active
February 10, 2017 15:57
-
-
Save Karkunow/be4da45395b1608649668a40f94e3019 to your computer and use it in GitHub Desktop.
Victor Kazak CR
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| ***************** | |
| * CODE PIECE #1 * | |
| ***************** | |
| https://github.com/area9innovation/sports/commit/a7fc77a3c4585e83c711979a25b0a59ef9b00102 | |
| insertOrUpdateModule(jwt, parentId, m, onOk : (int) -> void /*onOk is your fulfill*/) -> void { | |
| module = Module(m.id, parentId, m.topic, m.priority, m.sbId); | |
| - println("Insert or update module"); | |
| - updateSmartBuilderModule(jwt, module, \id -> | |
| - if (id == 0) | |
| - insertSmartBuilderModule(jwt, module, \aid -> { println("Insert module"); onOk(aid); }, nop1) | |
| - else { | |
| - println("Update module"); | |
| - onOk(id); | |
| - }, | |
| - println | |
| - ); | |
| + if (m.id == makeModule().id) | |
| + insertSmartBuilderModule(jwt, module, \id -> {println("Insert module"); onOk(id)}, println) | |
| + else | |
| + updateSmartBuilderModule(jwt, module, \id -> {println("Update module"); onOk(id)}, println) | |
| +} | |
| + | |
| +syncModulesRec(jwt : string, modulesInDb : [Module], parentId : int, lo : LO, onOk : (int) -> void) -> void { | |
| + | |
| + setIds = \m, id -> Module(id, parentId, m.topic, m.priority, m.sbId); | |
| + | |
| + module = switch (lo) { | |
| + GenericModule(m, __): m; | |
| + Module(__, __, __, __, __) : lo | |
| + }; | |
| + dbModule = findDef(modulesInDb, \m -> m.sbId == module.sbId, makeModule()); | |
| + identifiedModule = setIds(module, dbModule.id); | |
| + syncChildrenModules = \c, id -> doneP( | |
| + map(c, \ch : LO -> | |
| + Promise(\fulfill, reject -> syncModulesRec(jwt, modulesInDb, id, ch, fulfill)) | |
| + ) |> parallelP, | |
| + \arr -> { println("Promise fulfilled"); onOk(1); }, // case all are done, I don't know how do we use arr | |
| + println // case someone fail | |
| + ); | |
| + if (identifiedModule != dbModule) { | |
| + switch (lo) { | |
| + Module(__, __, __, __, __) : insertOrUpdateModule(jwt, parentId, identifiedModule, onOk); | |
| + GenericModule(__, c) : insertOrUpdateModule(jwt, parentId, identifiedModule, \id -> syncChildrenModules(c, id)) | |
| } | |
| + } | |
| + else { | |
| + switch (lo) { | |
| + Module(__, __, __, __, __) : onOk(dbModule.id); | |
| + GenericModule(__, c) : syncChildrenModules(c, dbModule.id) | |
| + } | |
| + } | |
| + | |
| } | |
| ***************** | |
| * CODE PIECE #2 * | |
| ***************** | |
| https://github.com/area9innovation/sports/commit/dc7d475fceacd92531046b8b20035f05f8611a36#diff-30e9e7051a0c322ef92f173b95c4bbb9R63 | |
| - upcomingSessions = filter(getDBTableData(state.dbState.sessionsTable), \s -> s.date >= stamp2date(timestamp())) | |
| - |> (\arr -> sortCustom(arr, \session -> session.date, false)); | |
| - previousSessions = filter(getDBTableData(state.dbState.sessionsTable), \s -> s.date < stamp2date(timestamp())) | |
| - |> (\arr -> sortCustom(arr, \session -> session.date, false)); | |
| + upcomingSessions = makeDerivativeTable(FilterTable(state.dbState.sessionsTable, \s -> s.date >= stamp2date(timestamp())), \s -> s.id); | |
| + previousSessions = makeDerivativeTable(FilterTable(state.dbState.sessionsTable, \s -> s.date < stamp2date(timestamp())), \s -> s.id); | |
| ... | |
| - makeListItem = \index -> { | |
| - learner = getDBTableByIdDef(state.dbState.learnersTable, sessions[index].learnerId, makeLearner()); | |
| - creationDate = sessions[index].date; | |
| - onClick = \ -> sessionOnClick(sessions[index]); | |
| + sessionItemsB = make([]); | |
| + getTableData(sessionsTable, ref makeTree(), \sessions -> next(sessionItemsB, map(reverseA(sessions), \s -> SessionItem(s, make(false)))), println); | |
| + insertsHandler = \changes -> { | |
| + insertToSessionItemsB = \session -> next(sessionItemsB, concat(v2a(SessionItem(session, make(true))), getValue(sessionItemsB))); | |
| + iter(changes, \change -> | |
| + switch (change) { | |
| + Insert(item) : insertToSessionItemsB(item); | |
| + default : {} | |
| + } | |
| + ) | |
| + } | |
| + | |
| + makeListItem = \sessionItem -> { | |
| + session = sessionItem.session; | |
| + learner = getDBTableByIdDef(state.dbState.learnersTable, session.learnerId, makeLearner()); | |
| + creationDate = session.date; | |
| + onClick = \ -> sessionOnClick(session); | |
| ... | |
| - mapi(sessions, \i, __ -> makeListItem(i)) | |
| - |> (\arr -> interleaveWithIndexes(arr, transformSeparator)) | |
| - |> MLines | |
| - |> (\m -> (if (length(sessions) == 0) noSessionsContent else m)) | |
| + MSelect(sessionItemsB, \sessions -> | |
| + mapi(sessions, \i, session -> makeListItem(session)) | |
| + |> MLines | |
| + |> (\m -> (if (length(sessions) == 0) noSessionsContent else m)) | |
| + ) | |
| + |> (\m -> makeCustomDataView(m, sessionsTable, insertsHandler)) | |
| + |> (\cdv : CustomDataView -> cdv.display) | |
| +} | |
| + | |
| +rippleItem(manager : MaterialManager, content : Material, trigger : DynamicBehaviour<bool>) -> Tropic { | |
| + box = make(WidthHeight(0.0, 0.0)); | |
| + point = fselect(box, FLift(\wh -> Point(wh.width / 2., wh.height / 2.))); | |
| + hover = make(false); | |
| + deltaTimer = manager.manager.deltaTimer; | |
| + if (getValue(trigger)) { | |
| + next(trigger, false); | |
| + deferred(\ -> next(trigger, true)); | |
| + } | |
| + disposer = subscribe2(trigger, \tr -> if (tr) timer(700, \ -> next(trigger, false))); | |
| + | |
| + animation = rippleAnimation2(manager, true, MGrey(500) |> const, const(0.4), point, trigger, const(false), box, rippleRadiusFill, rippleOffsetFill, false); | |
| + TAttachBoxCopy(material2tropic(manager, content), box) | |
| + |> (\t -> TGroup2(t, animation)) | |
| + |> (\t -> TDispose([disposer], t)) |
Author
Author
2.
Строки 66,67, 74, 76, 115 - слишком длинные. Либо делай разбивку аргументов по строкам. Либо называй аргументы и выноси их из вызова функции.
Строка 115 Функции с bool аргументами должны рефакториться. Они нечитабельны + это нарушает SRP. Если bool-аргументов много - вводи стили, которые будуть иметь понятные названия.
Строки 113-118 Почитай о /flow/doc/development.html#leaks. Все подписки должны быть внутри конструктора. Поэтому нужно использовать связку fconnect + fselect или connectSelectu функцию.
Строки 116-118 В данном случае значительно лучше выглядела бы версия без pipeline оператора.
Также отмечу, что используя pipeline - нужно давать короткие имена функциям в нем и использовать каррирование. Чтобы код в итоге выглядел как то так:
userList = data
|> initialProcessing
|> getUsersFromData
|> MUserList;
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1.
Строка 4: ненужный комментарий. Зачем о callback'e писать, что это callback? Я знаю, что это не твой код, но он находился рядом с местом редактирования и я хотел упомянуть, что - стоит обращать внимание на небольшие локальные недостатки, которые могут не иметь отношения к твоему текущему кейсу, и исправлять их в его же рамках. Таким образом код на проекте, хоть и маленькими порциями, но будет становиться лучше и лучше.
Строка 16: Для повышения читабельности нужно бы давать названия вот таким конструкциям - m.id == makeModule().id.
В этом случае нужно бы просто было сделать функцию isNewModule(module) -> bool и вынести логику туда.
Строки 38-39: Во-первых, ненужные комментарии. Все должно быть и так понятно по коду + функция onOk у тебя здесь имеет неправильный тип - она должна принимать массив id модулей, а не одну id, насколько я понял. Пересмотри код.
Строки 28, 42, 48: указывают на то, что у нас данная функция - SyncModulesRec делает 2-е вещи, а по Single Responsibility Principle (https://en.wikipedia.org/wiki/Single_responsibility_principle) мы должны бы иметь 2-е функции для этого. Одну для Module, вторую для GenericModule. Нужно отрефакторить.