HW 01 Bash by Geckostya · Pull Request #1 · Geckostya/SoftwareDesign

Geckostya · 2019-02-05T18:26:22Z

Парсер и команды в чём-то имеют несколько другую семантику, но, судя по заданию, это не страшно

ArtyomLobanov

Добавьте интеграцию с Travis
Обрабатывайте ошибки аккуратнее.
cat fghjftgh | wc
выводит информацию о количестве строк/слов/байт в сообщении об ошибке, а пользователь так и не узнает, что опечатался в имени файла. Не стесняйтесь кидать исключения. Их не просто так придумали.
На одинокий пайп в конце команды интерпретатор не ругается.
cat f.txt |

ArtyomLobanov · 2019-02-20T15:31:49Z

bash/build.gradle

+}
+
+dependencies {
+    compile "org.jetbrains.kotlin:kotlin-stdlib-jdk8"


А JUint из воздуха возьмётся?

ArtyomLobanov · 2019-02-20T15:35:03Z

bash/build.gradle

+repositories {
+    mavenCentral()
+}
+


Собирающийся jar файл нельзя запустить т.к. не указана точка входа в программу

ArtyomLobanov · 2019-02-21T09:03:34Z

bash/src/main/kotlin/hse/nedikov/bash/Parser.kt

+
+  private var state = Identifier
+  private var sb = StringBuilder()
+  private var vsb = StringBuilder()


Не очень говорящие названия. Называть переменные лучше по их смыслу, а не типу.

У них смысл строки собирать, что поделать, чисто утилитарные штуки. Названия лучше я не придумал, но расписал подробнее хоть

ArtyomLobanov · 2019-02-21T09:07:17Z

bash/src/main/kotlin/hse/nedikov/bash/Parser.kt

+
+    fun isIdentifier(c: Char): Char = if (c in 'a'..'z' || c in 'z'..'Z' || c == '_' || c == '-') c else '_'
+
+    fun isDigit(c: Char): Char = if (c in '0'..'9') c else '0'


А почему метод, начинающийся на is, возвращает не boolean?

Для матча, переименовал is в match

ArtyomLobanov · 2019-02-21T09:12:53Z

bash/src/main/kotlin/hse/nedikov/bash/logic/commands/Cat.kt

+      } catch (e: Exception) {
+        output.write("cat: ${e.message}")
+      }
+      output.write("\n")


System.lineSeparator()

ArtyomLobanov · 2019-02-21T09:36:37Z

bash/src/main/kotlin/hse/nedikov/bash/logic/commands/OuterCommand.kt

+  private fun startProcess(process: Process, output: PipedWriter) {
+    val streamGobbler = StreamGobbler(process.inputStream) { s -> output.write("$s\n") }
+    val errorGobbler = StreamGobbler(process.errorStream) { s -> output.write("$s\n") }
+    val executor = Executors.newSingleThreadExecutor()


Что-то как-то вы тут всё усложнили. Это было необходимо? В любом случае, Executor надо завершать

Я, естественно, не знал от рождения, как на джаве запускать команды кроссплатформенно, поэтому такое решение нагуглил

ArtyomLobanov · 2019-02-21T09:38:48Z

bash/src/main/kotlin/hse/nedikov/bash/logic/commands/WordCount.kt

+    val result = WCResult()
+    input.forEachLine {
+      result.lines++
+      result.words += it.split(' ').size


Плохой разделитель. Подумайте, когда это приводит к ошибкам.

ArtyomLobanov · 2019-02-21T09:39:53Z

bash/src/main/kotlin/hse/nedikov/bash/logic/commands/WordCount.kt

+    input.forEachLine {
+      result.lines++
+      result.words += it.split(' ').size
+      result.bytes += it.toByteArray().size


Окончания строк не учитываются.

ArtyomLobanov · 2019-02-21T09:48:46Z

bash/README.md

+
+Сначала строка из stdin попадает в парсер, где разбивается на токены
+
+Оттуда токены передаются в лексер вместе с окружением, где по пайпам токены делятся на команды: первый токен - имя, остальные - аргументы


Как-то у вас всё с ног на голову перевернулось. Обычно лексер разбивает текст на маркированные токены, а парсер строит AST. Я бы у вас названия местами поменял.

Мой мир перевернулся, почему-то у меня до этого успело сложиться впечатление, что я всё называю своими именами, а тут вот как. Хотя по факту можно и то и другое парсером назвать, но всё же лексер более подходит

ArtyomLobanov · 2019-02-21T09:55:18Z

bash/src/main/kotlin/hse/nedikov/bash/Controller.kt

+  }
+}
+
+private fun makeFlow(commands: ArrayList<Command>) =


Мы же хотим расширяемую и поддерживаемую архитектуру? Давайте создадим отдельный класс Интерпретатор, который будет содержать всю эту логику, а на вход принимать лексер, парсер и, например, входной и выходной потоки. Это позволит в будущем легко подменять налету отдельные части интерпретатора, создавать подпроцессы и т.д.

ArtyomLobanov · 2019-03-14T15:45:00Z

bash/src/main/kotlin/hse/nedikov/bash/logic/commands/WordCount.kt

+        throw Exception("wc: ${e.message}" + System.lineSeparator())
+      }
+    }
+    output.write("${result.lines} ${result.words} ${result.bytes} total" + System.lineSeparator())


Всегда выводит 0 0 0 т.к. переменная result нигде не модифицируется

ArtyomLobanov · 2019-03-14T15:51:41Z

bash/src/main/kotlin/hse/nedikov/bash/logic/commands/WordCount.kt

+    val result = WCResult()
+    input.forEachLine {
+      result.lines++
+      result.words += it.split(Regex("\\s+")).size


Теперь другая проблема: " some text ".split("\\s+") вернёт ["", "some", "text"]

ArtyomLobanov · 2019-03-14T15:54:48Z

bash/src/main/kotlin/hse/nedikov/bash/logic/commands/WordCount.kt

+    input.forEachLine {
+      result.lines++
+      result.words += it.split(Regex("\\s+")).size
+      result.bytes += (it + System.lineSeparator()).toByteArray().size


У последней строки может и не быть перевода строки. Для определения размера файла есть стандартные методы. Например, File.length Не надо изобретать велосипед.

ArtyomLobanov · 2019-03-15T10:01:24Z

bash/src/main/kotlin/hse/nedikov/bash/logic/commands/WordCount.kt

+  private fun calcInput(input: Reader): WCResult {
+    val result = WCResult()
+    val text = input.readText()
+    result.lines = text.split("\r\n|\r|\n").size


Либо преобразуйте к регэкспу, либо через перечисление

ArtyomLobanov · 2019-03-15T10:07:27Z

bash/src/main/kotlin/hse/nedikov/bash/logic/commands/OuterCommand.kt

+    val executor = Executors.newSingleThreadExecutor()
+    executor.submit(streamGobbler)
+    executor.submit(errorGobbler)
+    executor.shutdown()


Теперь внешние команды вообще не запускаются. Вы хоть тестируйте, прежде чем пушить в репозиторий.
Я предлагал просто использовать waitFor без аргументов

Ссори, постараюсь ответственнее относиться к этому

Geckostya · 2019-03-16T11:57:08Z

@ArtyomLobanov починил

ArtyomLobanov · 2019-03-21T10:23:14Z

Ок. @yurii-litvinov

yurii-litvinov

Мне всё нравится, зачтена

yurii-litvinov · 2019-03-24T18:21:07Z

bash/src/main/kotlin/hse/nedikov/bash/Controller.kt

+import java.io.PipedWriter
+
+
+class Controller(val exceptionHandler: (Exception) -> Unit) {


Вообще, каждому классу нужны комментарии

Geckostya added 8 commits January 29, 2019 20:23

base functionality

d17aeb6

Merge branch 'master' into bash

961f4f7

add graddle wrapper

f6221f3

refactor

733aab5

tests

aae09ec

Create README.md

5dba122

add tests

4f82697

add javadocs and a bit refactor

c3396ba

ArtyomLobanov suggested changes Feb 21, 2019

View reviewed changes

Geckostya added 3 commits March 1, 2019 14:44

travis

63b021f

finally I fix it

6d0bf64

fix readme

29c8690

ArtyomLobanov suggested changes Mar 14, 2019

View reviewed changes

Geckostya added 2 commits March 14, 2019 22:58

fix WordCount wrong results

e5a8b58

remove waiting 10 seconds from outer commands

b1f3a87

ArtyomLobanov suggested changes Mar 15, 2019

View reviewed changes

fix

92b7fdc

yurii-litvinov approved these changes Mar 24, 2019

View reviewed changes


		fun isIdentifier(c: Char): Char = if (c in 'a'..'z' \|\| c in 'z'..'Z' \|\| c == '_' \|\| c == '-') c else '_'

		fun isDigit(c: Char): Char = if (c in '0'..'9') c else '0'


		Сначала строка из stdin попадает в парсер, где разбивается на токены

		Оттуда токены передаются в лексер вместе с окружением, где по пайпам токены делятся на команды: первый токен - имя, остальные - аргументы

		import java.io.PipedWriter


		class Controller(val exceptionHandler: (Exception) -> Unit) {

Conversation

Geckostya commented Feb 5, 2019

Uh oh!

ArtyomLobanov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ArtyomLobanov Feb 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Geckostya commented Mar 16, 2019

Uh oh!

ArtyomLobanov commented Mar 21, 2019

Uh oh!

yurii-litvinov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ArtyomLobanov Feb 21, 2019 •

edited

Loading