julia_v_iluhina

Untitled

Nov 12th, 2016
80
0
Never
Not a member of Pastebin yet? Sign Up, it unlocks many cool features!
Java 13.75 KB | None | 0 0
  1. @app.route('/todo/api/v1.0/tasks', methods=['POST'])
  2. @auth.login_required
  3. def create_task():
  4.     ...
  5.         'id': len(tasks) + 1,
  6.     ...
  7. /*
  8.     у такой реализации - есть недостатки
  9.     пример
  10.     добавили 3 таски, удалили вторую
  11.     добавляем новую
  12.     результат - у двух тасок id = 3
  13.  
  14.     что не очень хорошо
  15.  
  16.     лучше проверь - если нет ни одной таски - пусть ид = 1
  17.     иначе - как раньше было = tasks[-1]['id'] + 1
  18.  
  19.     так будет универсальнее
  20.         https://lancelote.gitbooks.io/intermediate-python/content/book/ternary_operators.html
  21.         https://lancelote.gitbooks.io/intermediate-python/content/book/comprehensions.html (dict абстракции)
  22.         http://www.diveintopython.net/native_data_types/index.html#odbchelper.dict
  23. */
  24. ***********************************
  25.  
  26. http://joxi.ru/Y2LXgYnfnyRoX2
  27. /*
  28.     Имя пекеджа - поправь букву
  29.     ресурс - это только TasksRest
  30.     остальное - универсальные вещи - переноси в core - http://joxi.ru/E2pdR1lFB5Kx52
  31. */
  32. *******************************
  33. public class Configuration {
  34.  
  35.     public static String baseUrl = "http://localhost:5000/todo/api/v1.0/tasks";
  36.  
  37.     public static String credentials = "miguel:python";
  38. }
  39. /*
  40.     а вот это - смесь)
  41.     смесь универсального и уже контекста ресурса
  42.  
  43.     "miguel:python" - поскольку только такой способ аутентификации предполагается
  44.     то можно зашивать внутри метода ресурса (переменной не нужно - т к это нам понадобится лишь единожды)
  45.  
  46.     если бы был вариант аутентификации - разными методами
  47.     то "miguel:python" - это были бы уже тестовые даные
  48.     и мы бы размещали их в ветке проекта src \ test
  49.  
  50.     baseUrl - лучше было бы baseUri - точнее
  51.     и оставь значение = http://localhost:5000
  52.     именно эта часть uri зависит от того - где наш рест сервер размещен
  53.  
  54.     а в классе - ресурсе - уже уточни
  55.     public static String uri = Configuration.baseUri + "/todo/api/v1.0/tasks";
  56.     и им уже оперируй
  57.  
  58.     в таком варианте - если меняется расположение сервера - мы меняем только Configuration.baseUri
  59.     остальное - не трогаем
  60. */
  61. ************************
  62. public static Invocation.Builder authorized(Invocation.Builder requestBuilder) {
  63. /*
  64.     лучше быть точнее
  65.     и в универсальный метод  - передавать с помощью параметров все для его работы
  66.  
  67.     именно из этих соображений мы не работаем в пейджах напрямую с тестовыми данными
  68.     и тут логика похожая - в более универсальных методах мы не оперируем менене универсальными значениями напрямую
  69.     а используем параметры - для передачи в метод таких вещей
  70.  
  71.     именно так мы и обеспечиваем универсальность)
  72.     да и вызов - authorized("miguel:python", requestTo(uri));
  73.     более понятный
  74.  
  75.     public static Invocation.Builder authorized(String credentials, Invocation.Builder requestBuilder) - будет грамотнее
  76.     это для универсального RestHelpers
  77.  
  78.     а вот уже в TasksRest - реализуй уточненный
  79.         public static Invocation.Builder authorizedRequest(String uri)
  80.     в котором уточняйся до "miguel:python" и используй  authorized из RestHelpers
  81.     и его переиспользуй в других методах TasksRest
  82. */
  83. *********************
  84.  public static final Task[] DEFAULT_TASKS(String description, boolean done, String title, int id) {
  85.         Task[] DEFAULT_TASKS = new Task[2];
  86.         DEFAULT_TASKS[0] = new Task("Milk, Cheese, Pizza, Fruit, Tylenol", false, "Buy groceries", 1);
  87.         DEFAULT_TASKS[1] = new Task("Need to find a good Python tutorial on the web", false, "Learn Python", 2);
  88.         return DEFAULT_TASKS;
  89.     }
  90.  
  91.  public static final Task[] DEFAULT_TASKS = DEFAULT_TASKS(description, done, title, id);
  92. /*
  93.     это - тестовые данные
  94.     не нужно их зашивать в классе-ресурсе
  95.     подобно тому - как мы не зашивали в пейджах тестовые данные
  96.  
  97.     если тебе эти данные понадобятся в нескольких тест-классах - то вынесешь это в отдельный класс
  98.     (разместив в ветке src \ test)
  99.  
  100.     у нас такой проблемы нету
  101.     размещай эти данные к тест-классе
  102.  
  103.     и можно это делать проще
  104. */
  105.     public final Task[] DEFAULT_TASKS = {
  106.             new Task(...),
  107.             new Task(...)
  108.     };
  109. *****************************************
  110.     private static Response response;
  111.  
  112.     private static String description;
  113.     private static String title;
  114.     private static boolean done;
  115.     private static int id;
  116.  
  117.     public static int getResponseStatus() {
  118.     public static String getErrorAnswer() {
  119.     public static Task getResponseTask() {
  120.  
  121. /*
  122.     вот эти переменные и методы - точно не нужны
  123.  
  124.     переменные description, title, done,  id - тут не используются
  125.     да и это свойства таски
  126.     и класс-ресурс к этому не имеет отношения
  127.  
  128.      про response - писала в прошлый раз строки 112 - 170
  129.      еще уточню
  130.      туда же - и методы для работы с response
  131.  
  132. */
  133. **************************************
  134.     public static int getTasksSize() {
  135.         return getTasks().size();
  136.     }
  137.  
  138.     public static Task getTask(int id) {
  139.         return getTasks().get(id);
  140.     }
  141. /*
  142.     для получения размера или таски - мы заново делаем запрос
  143.     тоже - не наш метод)
  144.     слишком неэффективно
  145.     будем избавляться
  146. */
  147. **************************************************
  148.     public static void getUnauthorizedAccess() {
  149.     public static void getAuthorizedAccess() {
  150. /*
  151.     метод getAuthorizedAccess() - можно назвать проще - get()
  152.     да и сразу реализовать возвращающим List<Task>
  153.     тогда и метод getTasks() - не будет нужен )
  154.  
  155.     TasksRest - это полноценный ресурс-модуль, который предоставляет CRUD
  156.  
  157.     я бы сэкономила на названиях методов)
  158.     и вызывала методы ресурса  - TasksRest.get()
  159.     и точно, и понятно
  160.  
  161.     метод getUnauthorizedAccess() - не уверена - стоило ли реализовывать
  162.     реально - это нужно лишь единожды
  163.     когда нужно проверить собственно  - UnauthorizedAccess )
  164.     остальное - работает только при авторизации
  165.  
  166.     а раз нужно лишь единожды - так зачем тогда метод?
  167.  
  168.     если все же решишь оставить метод getUnauthorizedAccess() в ресурсе - то имя тоже подравняй
  169.     unauthorizedGet - отвечает выше предложенной логике, но тоже ... мне не нравится
  170.     вот сравни код
  171.     Response response = TasksRest.unauthorizedGet();
  172.     и
  173.     Response response = requestTo(TasksRest.uri).get();
  174.  
  175.     Мы ж вообще ничего не выиграли
  176.     ни в наглядности
  177.     ни в DRY (нужно один раз)
  178.  
  179.     так что - очень рекомендую - удалить из ресурса метод getUnauthorizedAccess()
  180. */
  181. **************************************************
  182.     public static void createTask(Task task) {
  183.         response = authorized(requestTo(Configuration.baseUrl)).post(Entity.entity(task, MediaType.APPLICATION_JSON));
  184.     }
  185. //сравни
  186.    public static Task create(Task task) {
  187.         Response response = authorizedRequest(uri).post(Entity.entity(task, MediaType.APPLICATION_JSON_TYPE));
  188.         assertEquals(201, response.getStatus());
  189.         return response.readEntity(TaskContainer.class).getTask();
  190.    }
  191. /*
  192.     вспомни - я выше в этом ревью писала - почему не нужна переменная response в классе-ресурсе
  193.  
  194.     при такой реализации - мы в методах класса-ресурса - проверяем статус ответа
  195.     и уже возвращаем - данные из ответа
  196.  
  197.     пересмотри прошлое ревью про это
  198.  
  199.     если кажется это слишком "магическим" вариантом
  200.     то возвращай из метода Response
  201.     а проверку статуса ответа и получение данных - делай в тест-методе
  202.  
  203.     если выбирать  "магию" - то лучше максимально полезную
  204.     в том смысле - которая дает явные выигрыши какие-то
  205.  
  206.     в текущем твоем варианте - много всего в классе-ресурсе есть
  207.     много сущностей, зависимых друг от друга
  208.     и тут надо, как в оркестровом музыкальном произведении - правильно этим всем пользоваться
  209.  
  210.     вариант, что я предложила - значительно проще
  211.     сделали запрос + сразу проверили статус ответа + вернули данные из запроса
  212.  
  213. */
  214. *************************************************
  215.  public static void createTask(Task task) {
  216.  public static void updateTask(Task task) {
  217.  public static void deleteTask(int id) {
  218. /*
  219.     имена этих методов можно сделать полаконичнее
  220.     create
  221.     update
  222.     delete
  223.  
  224.     Вызывая их как TasksRest.create(...) - получишь вполне наглядный код
  225.  
  226.     и каждый из этих методов подправь
  227.     все методы реализуй по схеме
  228.     сделали запрос + сразу проверили статус ответа + вернули данные из запроса(если есть что вернуть, конечно)
  229. */
  230. *************************************************
  231.     public static void assertTasks(Task... tasks) {
  232.         List<Task> expectedTasks = new ArrayList<Task>();
  233.         for (Task task : tasks) {
  234.             expectedTasks.add(task);
  235.         }
  236.  
  237.         List<Task> actualTasks = getTasks();
  238.  
  239.         for (int i = 0; i < expectedTasks.size(); i++) {
  240.             assertTasksAreEqual(expectedTasks.get(i), actualTasks.get(i));
  241.         }
  242.     }
  243.  
  244.     public static void assertTasksAreEqual(Task expectedTask, Task actualTask) {
  245.  
  246.         assertEquals(expectedTask.getDescription(), actualTask.getDescription());
  247.         assertEquals(expectedTask.isDone(), actualTask.isDone());
  248.         assertEquals(expectedTask.getTitle(), actualTask.getTitle());
  249.         assertEquals(expectedTask.getUri(), actualTask.getUri());
  250.     }
  251. /*
  252.     преобразование Task... tasks к списку тасок - можно реализовать проще - Arrays.asList(tasks)
  253.  
  254.     задача сравнения объектов класса Task - должна решаться самим классом Task
  255.     я не зря приводила линки
  256.          http://www.java2s.com/Tutorial/Java/0140__Collections/CheckingforEquality.htm
  257.          http://www.javapractices.com/topic/TopicAction.do?Id=17
  258.          http://javadevnotes.com/java-array-to-list-examples
  259.     в рамках класса  Task - реализуй методы -
  260.         public boolean equals(Object o)
  261.         public int hashCode()
  262.     вот на эту линку особое внимание обрати - http://www.javapractices.com/topic/TopicAction.do?Id=17
  263.     IntelIJ Idea помогает в этом - используй контекстное меню в коде класса - generate...
  264.  
  265.     а чтобы сообщение об ошибке при сравнении было наглядным - реализуй в классе Task
  266.     метод toString() - тоже можно использовать  generate...
  267. */
  268. ************
  269. public Task(String description, boolean done, String title, int id)
  270. /*
  271.     я бы на уровне конструктора класса - оперировала не параметром int id
  272.     а уже uri
  273.    
  274.     цель - разделить логику
  275.    
  276.     пусть контейнеры ничего не знают о том, как мы работаем с uri
  277. */
Advertisement
Add Comment
Please, Sign In to add comment