-
Notifications
You must be signed in to change notification settings - Fork 14
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
hw-01 #3
base: main
Are you sure you want to change the base?
hw-01 #3
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job, but pay attention on some points I marked, and try to code more Clojure-way :)
; (if (> i n) | ||
; r | ||
; (loop [j 0 s nil] | ||
; (if (< j n) (recur (+ 1 j) (conj s [i j])) s))))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inc
is more idiomatic than (+ 1 ..
but it's a style case
(if (= s1-elem s2-elem) (assoc-in t [i j] (+ i-1j-1 1)) (assoc-in t [i j] (max i-1j ij-1))))) | ||
t [(vec (repeat (+ 1 (count s1)) 0))] | ||
r (vec (repeat (+ 1 (count s2)) [0])) | ||
table (vec (concat t r)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may use maps instead of vectors for DP, and then you don't need to prefill it by zeros, all getters has variants with defaults (get-in {} [1 2] 33) => 33
(defn aux [s len pos] | ||
(cond | ||
(>= pos len) true | ||
(= (nth s pos) (nth s (- len pos))) (aux s len (+ 1 pos)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no such thing as TCO in Clojure, so recur
would be better
|
||
(def letters | ||
[\a \b \c \d \e \f \g \h \i \j \k \l \m \n \o \p \q \r \s \t \u \v \w \x \y \z]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(vec "abcdefghijklmnopqrstuvwxyz")
:)
(def chars-set (set (map char (range (int \a) (+ (int \a) 26))))) | ||
|
||
(defn is-pangram [test-string] | ||
(>= (count (reduce (fn [a e] (conj a e)) (set []) (str/lower-case test-string))) 26) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(fn [a e] (conj a e))
== conj
:)
(set [])
== #{}
:)
(reduce (fn [a e] (conj a e)) (set []) xxx)
== (set xxx)
:)))
PS: you'll reduce all the string, even if it's already pangram on it's first 2%
PPS: "0123456789!@#$%^&*()_+|~<>,." will be pangram by your code :)
(defn normalize-text [text] | ||
(-> text | ||
(clojure.string/replace #"\s+|\p{Punct}" "") | ||
clojure.string/lower-case)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
str/lower-case
. You already imported qualified ns on line 3 of this file
@@ -1,6 +1,17 @@ | |||
(ns otus-02.homework.pangram | |||
(:require [clojure.string :as string])) | |||
(:require [clojure.string :as string] | |||
[clojure.string :as str])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bad practice, just one import for 1 ns
transposed (map (fn [i] (map #(nth % i \space) a)) (range n)) | ||
res (remove-spaces (apply str (map (fn [elem] (apply str (map str elem))) transposed)))] | ||
res)) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may work, but.... Looks like you try to use imperative algorithms with mutable state in a case of Clojure, where we prefer immutable data and functional way.
Hope we'll discuss it on Q&A stream
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Хорошо, заметен прогресс. Некоторые моменты прокомментировал
(with-open [file (FileInputStream. file-path) | ||
buffer (BufferedInputStream. file)] | ||
(let [file-size (.available file) | ||
byte-array (byte-array file-size)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Вот так затенять именами переменных имена функций стандартной либы чревато тем, что в динамически типизированном языке при компиляции оно тебе не покажет ошибку, но потом в рантайме свалится, когда ты забудешь и ниже по коду вызовешь эти библиотечные функции по имени. Сто раз уже нарывались на это с name
, first
и т.п.
bytes)) | ||
|
||
(defn change-order [[b1 b2 & rest]] | ||
(if (nil? rest) (conj rest b1 b2) (conj (change-order rest) b1 b2))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Не будет стековерфлоу на больших байтовых массивах?
(String. (byte-array byte-vector) "UTF-8")) | ||
|
||
(defn parse-content [enc buffer] | ||
(cond |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
case enc
?
|
||
(defmulti decode-frame dispatch-frame) | ||
|
||
(defmethod decode-frame "TALB" [frame] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[{:keys [content] :as frame}]
(defmulti decode-frame dispatch-frame) | ||
|
||
(defmethod decode-frame "TALB" [frame] | ||
(merge frame {:content (str "Album: " (:content frame))})) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
update frame :content #(str "Album: " %)
size (decode-synchsafe size-v) | ||
[flags r3] (split-at 2 r2) | ||
[enc r4] (split-at 1 r3) | ||
content (parse-content (first enc) (take (- size 1) r4))] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(dec size)
:enc enc | ||
:content content | ||
:rest (drop size r3) | ||
:rest-size (- total-size size 10)})) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Достаточно одного раза, побочных эффектов нет :)
(loop [buffer b | ||
total size | ||
frames []] | ||
(let [{rest :rest rest-size :rest-size content :content id :id enc :enc :as frame} (read-frame buffer total) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(let [{:keys [rest rest-size content id enc] :as frame}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Хорошо, к некоторым стилевым моментам придираться не стал :)
|
||
(defn gen [properties] | ||
(->> (keys properties) | ||
(reduce (fn [acc k] (assoc acc k (to-spec (get properties k)))) {}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Можно сразу (reduce (fn [acc [k v]]
на properties
- мапа переводится в сиквенс пар ключ-значение. Это если не выпендриваться с reduce-kv
:)
(def valid-types #{"string" "number" "boolean" "integer"}) | ||
|
||
(defn is-valid-spec [spec] | ||
(let [fields-object (get-in spec ["properties"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(get spec "properties")
, или (defn is-valid-spec [{:strs [properties] :as spec}]
(let [fields-object (get-in spec ["properties"]) | ||
fields (keys fields-object) | ||
types (map (fn [field] (get-in fields-object [field, "type"])) fields) | ||
are-types-valid (every? (fn [e] (contains? valid-types e)) types)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(every? valid-types types)
- сет имплементирует интерфейс IFn
(defn render-gen [parsed-spec] | ||
{:status 200 | ||
:headers {"Content-Type" "application/json"} | ||
:body (gen (get-in parsed-spec ["properties"]))}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Те же get или дистракчеринг
(defroutes app-routes | ||
(GET "/" request | ||
(let [params (:query-params request) | ||
spec (get-in params ["spec"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(let [{:strs [spec]} (:query-params request)
(GET "/" request | ||
(let [params (:query-params request) | ||
spec (get-in params ["spec"]) | ||
parsed-spec (json/parse-string spec)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Если не валится с ошибкой, когда spec == nil то ок
(def app | ||
(-> app-routes | ||
(wrap-reload) | ||
wrap-params |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Чем wrap-reload заслужил скобки, а wrap-params нет? :)
No description provided.