Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Домашка #13

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ManginAlexander
Copy link

^^

/*global _testElNotAscending: true*/
/*global _testElAscendingAndEqualsElement: true*/
/*global _runTestSortArray: true*/
/*global _testElNotAscendingAndEqualsElement: true*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

вместо глобалов можно было сначало объявить функцию, а потом ее использовать те runTestsSort опустить в самый конец скрипта

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Но тогда код будет неопрятным. Так как в начале файла, модуля, класса должно быть самое важное. Ну это на мой взгляд.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Код должен быть максимально самодокументированным. Используй тогда модуль заодно и от _ избавишься

var mySpecialFunction = (function () {

    function myPrivate() {

    }

    return function () {
        var stuff = myPrivate();

        return stuff * 42;
    };

}());

mySpecialFunction(100500);

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Я не понял комментария на счет "самодокументированности кода".
    Писать комментарии? - мне кажется это избыточно в данной ситуации (так как не очевидного кода нет) - DRY
    Писать тесты в которых описано как и что работает? - я это вроде бы написал в той или иной степени
    Писать вики документацию? - когда я устраивался на стажировку мой код с вики документацией "обсмеяли" - DRY.
  2. Если мы засунем все в функцию:
    • ограничили видимость функции
    • я не понимаю как протестировать "внутренние" функции.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Совсем не посмотрел на имена функций - все они тесты. Оставь как есть. Не заморачивайся :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

А можете выложить под тестовым пользователем, вашу версию данной задачи? хочется научиться чему, то хорошему.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Вот что бы я сделал по максимуму:

  • Сделал бы из файла модуль (LMD или AMD)
  • JSDoc для всех публичных функций (те, что на экспорт из модуля) с @example
  • Сделал бы из модуля npm-пакет - package.json (прописал бы все зависимости)
  • Добавил бы юнит-тестов на основе QUnit или Mocha test/*
  • Сделал бы автоматизированный и безбраузерный запускатель тестов на node.js и PhantomJS
  • Сделал бы автоматизированную проверку кода JSHint (не JSLint)
  • Предыдущие 2 должны запускаться по make test и npm test
  • Сделал бы git-hook на основе JSHint, который не давал бы мне коммитить говнокод (желательно это сделать сразу иначе потом забиваешь).
  • Сделал бы автоматизированную сборку документации make docs
  • Прописал бы проект в TravisCI для автоматизированного тестирования каждого моего коммита в git origin
    • TravisCI при каждом коммите скачивает к себе мой репозиторий выполняет npm install && npm test (вобщем автоматом ставит и запускает базбраузерные тесты)

Почти все это я сделал тут https://github.com/azproduction/lmd

1. Добавил документацию к публичному методу bubble search.
2. Переписал тесты c использованием QUnit
Удалил лишние файлы, перекодировал все в utf8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants