Feedback #1

github-classroom · 2023-03-23T05:28:47Z

👋! GitHub Classroom created this pull request as a place for your teacher to leave feedback on your work. It will update automatically. Don’t close or merge this pull request, unless you’re instructed to do so by your teacher.
In this pull request, your teacher can leave comments and feedback on your code. Click the Subscribe button to be notified if that happens.
Click the Files changed or Commits tab to see all of the changes pushed to main since the assignment started. Your teacher can see this too.

Notes for teachers

Use this PR to leave feedback. Here are some tips:

Click the Files changed tab to see all of the changes pushed to main since the assignment started. To leave comments on specific lines of code, put your cursor over a line of code and click the blue + (plus sign). To learn more about comments, read “Commenting on a pull request”.
Click the Commits tab to see the commits pushed to main. Click a commit to see specific changes.
If you turned on autograding, then click the Checks tab to see the results.
This page is an overview. It shows commits, line comments, and general comments. You can leave a general comment below.
For more information about this pull request, read “Leaving assignment feedback in GitHub”.

Subscribed: @SurfaceYellowDuck @ancavar @nemakin

linter action have to run on every push and pull_request.

…ree class

…g them

Bst

Red black

Revert "Red black"

Red black в dev

…rom any derived tree

* feat: Add AVL tree with own BST implementation

* Create LICENSE * fix: change insert method * fix: change tests * repo: add CI * feat: implement remove method and test for it

Co-authored-by: Nikita Lukonenko <68463602+surfaceyellowduck@users.noreply.github.com>

* ci: build on multiple os, remove redundant cache * refactor: better build.gradle.kts code style * ci: Add dependabot * fix: remove debugging tools * fix: change color of RBT node to enum type * fix: Make methods for rotation abstract, fix flaky AVL test with shuffling values and other minor fixes... * implemented saving data to sqlite db * feat: implemented reading tree object from sqlite database * feat:add docker compose for lift postgresql db and small fixes * fix: Rework tree deserialization logic * add bat, sh files to up docker container * add .gitignore * fix: rework neo4j interaction * fix: null safety * fix:remove the db files in folders * fix warning associated with sf4j * feat: Initialize dependencies for UI * fix readme and add setName method to all trees * fix:fix readme * feat: add the functionality of switching between views for different trees * feat: Add the display of the binary search tree, add the ability to insert a node and clear the tree. * feat: add mapping trees list from database * Add display of the tree to choose from the list * feat: Add deletion of BST from DB and list of trees by click, little trees refactoring * add ability to save bst in db with gui * feat: add ability to delete node * fix: fix a bug related to the fact that tables were not created when starting the application * feat: Add the ability to draw an AVl tree and work with it through the graphical interface * feat: Add RBTree visualization with ability to interact with Neo4j DB * feat: better text positioning * fix: fix build on mac * feat: add tooltip to show value * feat: add tree resizing when resizing app's window * refactor * fix: setup java 11 before building * fix: review code * fix: removed space in app/build.gradle.kts * add example gif for readme * fix:add link to gif in readme * chore: add comments to bst structure * chore: Add comments for DB controllers * fix: fix linter errors * fix: fix linter errors * chore: Add more comments to DB related stuff * feat: Add ability to draw colored nodes in RBTree * chore: add comments for app * fix: fix linter errors * fix: fix linter error * fix: Remove bug when error doesn't appear after inserting empty value twice * feat: add panning * feat: restrict resizing of window's app * fix: remove scrollbar * fix: remove unnecessary border * add json folder --------- Co-authored-by: Nikita Nemakin <nikita.nemakin@yandex.ru> Co-authored-by: SurfaceYellowDuck <lukonenko.nikitka@mail.ru>

Urtix

Итого: все неплохо, за исключением нескольких моментов.

Urtix · 2023-05-03T16:39:06Z

trees/src/main/kotlin/bst/AbstractBST.kt

+            } else if (node.right == null) {
+                return node.left
+            } else {
+                val tmp: Self = findMax(node.left)!!


Не заменили на ?: throw IllegalStateException("описание состояния")
Такой вариант использования !! приводит к непредвиденному NullPointerException.

да там просто такое чувство что ну невозможно null получить если всмотреться, все до этого проверили

Urtix · 2023-05-03T17:17:51Z

trees/src/main/kotlin/bst/db/controllers/JsonController.kt

+        } catch (e: Exception) {
+            println("Write error")
+        }


Так выводить ошибку при exception не стоит.
Лучше использовать throw IllegalArgumentException("Описание ошибки")

Urtix · 2023-05-03T17:19:39Z

trees/src/main/kotlin/bst/db/controllers/JsonController.kt

+        } catch (e: Exception) {
+            println("Read error")
+            null


Та же проблема с выводом exception

Urtix · 2023-05-03T17:22:34Z

trees/src/main/kotlin/bst/db/controllers/JsonController.kt

+    override fun removeTree(treeName: String) {
+        File("${this.folderPath}/$treeName.json").delete()
+    }


Тут бы тоже не помешала проверка на exeption.

Urtix · 2023-05-03T18:05:31Z

json/test.json

@@ -0,0 +1 @@
+{"treeName":"","rootNode":{"height":2,"key":234,"value":"gfh","left":{"height":1,"key":23,"value":"","right":{"height":0,"key":67,"value":"dgfh"}},"right":{"height":0,"key":324,"value":"fdsg"}}}


Имхо, имя файла стоило другое дать, а то сначала подумал что это для каких-то тестов.

мы просто не знали как пустую папку в репозитории создать / запушить, пришлось вместе с этим пушить

Urtix · 2023-05-03T18:50:58Z

А как запускать графический интерфейс то?

Urtix · 2023-05-03T19:33:30Z

app/src/main/kotlin/app/controller/RBTController.kt

+     * @return True if the string can be parsed as an integer, false otherwise.
+     */
+
+    fun isNumeric(s: String): Boolean {


isDigit есть у String

isDigit помоему у Char, но в целом метод можно было красивее написать, согласен

Urtix · 2023-05-03T19:35:11Z

app/src/main/kotlin/app/controller/RBTController.kt

+        if (root != null) {
+            drawNode(root, treePane, treePane.width / 2.0, treePane.width / 2.0, 250.0)
+        } else {
+            throw NullPointerException("Root of the tree is null.")


То есть приложение просто упадет, если какая-то внутренняя ошибка произойдет. Не очень user friendly. Наверное, если рут налл, то можно ничего не делать. Как вам идея?

Urtix · 2023-05-03T19:39:08Z

trees/src/main/kotlin/bst/db/controllers/JsonController.kt

+    /**
+     * The path to the folder where JSON files are stored.
+     */
+    val folderPath = "json"


А если очень хочу сохранить куда-то на компьютер? Бтв, неясно какая будет в итоге локация у файла после превращения в исполняемый файл

Urtix · 2023-05-03T19:40:44Z

trees/src/main/kotlin/bst/db/controllers/Neo4jController.kt

+     */
+    private val config = Configuration.Builder()
+        .uri("bolt://localhost")
+        .credentials("neo4j", "password")


Хочу другой neo4j использовать, например в вебе. Как мне это сделать? Кстати, это ломает приложение при запуске, если у пользователя не поднят докер с нужными портами. Очень странно, если хочется только json

Urtix · 2023-05-03T19:42:22Z

trees/src/test/kotlin/bst/AvlTest.kt

+import org.junit.jupiter.api.Test
+import kotlin.math.max
+
+class AvlTest {


AVL - аббревиатура. Должна писаться КАПСОМ

Urtix · 2023-05-03T19:44:24Z

По сравнению с предыдущим ревью все очень хорошо.
В большинстве своем читаемый код с нормальными комментами.
Жаль, что проект без докера не работает.

Urtix · 2023-05-03T19:46:59Z

Падает при попытке сохранить дерево без имени. Хотя и не полностью, но текст ошибки очень страшный

Urtix · 2023-05-03T19:48:05Z

Попытка удаления аналогично

Urtix · 2023-05-03T19:55:37Z

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

Urtix · 2023-05-03T20:03:51Z

Если в AVL дереве именем файла указывать сколько-то "/", то они все будут удаляться и будет просто .json. В итоге AVL дерево перестанет работать.

Urtix · 2023-05-03T20:10:36Z

Если дерево уже существует и его имя указать при работе с другим, то оно перезапишется. Где предупреждение, что дерево уже существует?

Urtix · 2023-05-03T20:17:25Z

С обратными слешами в AVL такая же проблема.

github-classroom bot and others added 30 commits March 23, 2023 05:28

Setting up GitHub Classroom Feedback

c684c27

chore: Add .gitignore

2440aca

chore: Set up project structure

3a6e826

fix(repo): Delete .idea dir and add proper .gitignore

551bf1b

feat(repo): add general project structure

4a156c0

Create linter action

d64f79b

linter action have to run on every push and pull_request.

feat: add BalancingTree class

f77fa40

add insert method and private recursiveInsert method to BinarySearchT…

04961ce

…ree class

add find method and private search method is BinarySearchTree class

1c09271

feat: implement rotations in BalancingTree

1c58d50

feat: implement insert method

095a1f3

refactor: remove overriding find and clear method as we are inheritin…

4ce4b0b

…g them

Merge pull request #2 from spbu-coding-2022/BST

4e46238

Bst

fix: remove redundant value parameter from remove method

81095ba

feat: test template for RBT

5779c31

refactor: major refactoring according to linter

18d2f20

refactor: minor refactoring

bec9900

Merge pull request #3 from spbu-coding-2022/red-black

6f0866a

Red black

Revert "Red black"

d70caab

Merge pull request #4 from spbu-coding-2022/revert-3-red-black

fb4350f

Revert "Red black"

Merge branch 'dev' into red-black

eb6ba6e

Merge pull request #5 from spbu-coding-2022/red-black

1d6ea04

Red black в dev

fix: remove value parameter from Tree interface

5314ceb

fix: change search function to protected and so that it can be used f…

2006ec6

…rom any derived tree

Create README.md

391e531

Implement AVL tree (#6)

cf8e545

* feat: Add AVL tree with own BST implementation

feat: implement better red-black tree methods and tests for it

ac0da2c

* Create LICENSE * fix: change insert method * fix: change tests * repo: add CI * feat: implement remove method and test for it

fix: band-aid fix for removal

54913ce

Delete .idea directory

ab4e10d

add readme from main

c95cf57

ancavar and others added 20 commits May 1, 2023 00:08

fix: null safety

4df2204

fix:remove the db files in folders

0ee304e

fix warning associated with sf4j

737623c

refactor: better test names, remove debugging info

ad89890

feat: add logger to neo4j

10ad5bb

refactor: ctrl+shift+alt+l all files

4a940a4

fix readme and add setName method to all trees

fc1ba27

refactor: better README.md

05bc89f

Co-authored-by: Nikita Lukonenko <68463602+surfaceyellowduck@users.noreply.github.com>

feat: bring back markdown linter

fa8124e

feat: add neo4j to docker-compose.yml

d764fe0

feat: add test coverage reports back

b44b5d7

fix: typos in README.md

6d0f73b

fix: better names, remove unnecessary cast

1438bcd

feat: implement Controller interface, add removal to JSON

cb8506b

fix: improve linter

f8c9dfd

fix: remove setName usage

50f9f7c

feat: add close session method

e270336

refactor

2d4ec8b

refactor: more

dcc1a30

Urtix suggested changes May 3, 2023

View reviewed changes

Urtix reviewed May 3, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feedback #1

Feedback #1

github-classroom bot commented Mar 23, 2023 •

edited

Loading

Urtix left a comment

Urtix May 3, 2023

ancavar May 3, 2023

Urtix May 3, 2023

Urtix May 3, 2023

Urtix May 3, 2023

Urtix May 3, 2023

ancavar May 3, 2023

Urtix commented May 3, 2023

Urtix May 3, 2023

ancavar May 3, 2023

Urtix May 3, 2023

Urtix May 3, 2023

Urtix May 3, 2023

Urtix May 3, 2023

Urtix commented May 3, 2023

Urtix commented May 3, 2023 •

edited

Loading

Urtix commented May 3, 2023

Urtix commented May 3, 2023

Urtix commented May 3, 2023

Urtix commented May 3, 2023

Urtix commented May 3, 2023 •

edited

Loading

		@@ -0,0 +1 @@
		{"treeName":"","rootNode":{"height":2,"key":234,"value":"gfh","left":{"height":1,"key":23,"value":"","right":{"height":0,"key":67,"value":"dgfh"}},"right":{"height":0,"key":324,"value":"fdsg"}}}

Feedback #1

Are you sure you want to change the base?

Feedback #1

Conversation

github-classroom bot commented Mar 23, 2023 • edited Loading

Urtix left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Urtix commented May 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Urtix commented May 3, 2023

Urtix commented May 3, 2023 • edited Loading

Urtix commented May 3, 2023

Urtix commented May 3, 2023

Urtix commented May 3, 2023

Urtix commented May 3, 2023

Urtix commented May 3, 2023 • edited Loading

github-classroom bot commented Mar 23, 2023 •

edited

Loading

Urtix commented May 3, 2023 •

edited

Loading

Urtix commented May 3, 2023 •

edited

Loading