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

Lab1 Rudnevskiy #6

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Lab1 Rudnevskiy #6

wants to merge 1 commit into from

Conversation

helytpro
Copy link

No description provided.

@alexeyr
Copy link
Owner

alexeyr commented Mar 29, 2024

Сделайте свой репозиторий приватным, пожалуйста.

median3 x y z | (min x z <= y) && (y <= max x z) = y --sum
| (min y z <= x) && (x <= max y z) = x
| (min x y <= z) && (z <= max x y) = z
| otherwise = error "the median has not been calculated"
Copy link
Owner

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.

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

Copy link
Owner

Choose a reason for hiding this comment

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

Да, но поэтому можно уже третий случай сделать otherwise.

@@ -47,7 +59,9 @@ rbgToCmyk color = error "todo"
-- используйте рекурсию
-- не забудьте случаи n < 0 и n == 0.
geomProgression :: Double -> Double -> Integer -> Double
geomProgression b q n = error "todo"
geomProgression b q n | (n == 0) = b
| n < 0 = error "n must be a natural number: n >= 2"
Copy link
Owner

Choose a reason for hiding this comment

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

Почему >= 2?

Copy link
Author

Choose a reason for hiding this comment

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

Лишнее условие. Я хотел поставить условие неотрицательности, но здесь достаточно n > 0

euclidAlgorithm x 0 = x
euclidAlgorithm x y = euclidAlgorithm y (mod x y)
in
if ((a * b) == 0) then error "ZeroException" else if euclidAlgorithm a b == 1 then True else False
Copy link
Owner

Choose a reason for hiding this comment

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

-1 и -1 взаимно простые?

Copy link
Author

Choose a reason for hiding this comment

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

Взаимно простые, но функция выдает ошибку. Связано это, предположительно, с особенностями обработки отрицательных чисел функцией mod. Возможное решение - брать от чисел их модули (что-то вроде функции abc в стандартных математических пакетах).

distance, abnormDistance :: Point -> Point -> Double

abnormDistance p1 p2
| not (length (coords p1) == length (coords p2)) = error "DifferentDimensionError"
Copy link
Owner

Choose a reason for hiding this comment

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

Такая проверка будет медленной и делаться у вас на каждом шаге рекурсии. Видно ли, как её можно избежать?

И во вторую очередь, как можно поменять тип abnormDistance, чтобы упростить код?

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
Owner

@alexeyr alexeyr Apr 5, 2024

Choose a reason for hiding this comment

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

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

find f [] = Nothing
find f xs
| (f (head xs)) = Just (head xs)
| not (f (head xs)) && (not (null (tail xs))) = find f (tail xs)
Copy link
Owner

Choose a reason for hiding this comment

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

not (f (head xs)) здесь лишнее; если бы оно не выполнялось, мы попали бы в первую ветвь. Соответственно, null (tail xs) в третьей ветви тоже.

Copy link
Author

Choose a reason for hiding this comment

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

Можно записать через if: if (f (head xs)) then Just (head xs) else find f (tail xs)

Copy link
Owner

Choose a reason for hiding this comment

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

Да, так тоже можно.

| not (f (head xs)) && (not (null (tail xs))) = find f (tail xs)
| null (tail xs) = Nothing

findLast f x | (length (filter f x)) > 0 = Just $ last (filter f x)
Copy link
Owner

Choose a reason for hiding this comment

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

Тут формально получается, что filter f x вычисляется 2 раза, хотя возможно, компилятор это оптимизирует. Как можно избежать этой проблемы?

Copy link
Author

Choose a reason for hiding this comment

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

Можно создать список элементов x, таких, что: filter f x > 0 , затем извлечь из него последний элемент

Copy link
Owner

Choose a reason for hiding this comment

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

Я имел в виду просто вынести filter f x в локальную переменную.

Comment on lines +90 to +91
| any (False ==) (mapFuncs preds x) = False
| otherwise = False
Copy link
Owner

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.

написать что-то в духе if (all (True ==) (mapFuncs preds x)) else False

Copy link
Owner

Choose a reason for hiding this comment

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

Тут имелось в виду, что если два условия с одинаковым результатом, и последнее otherwise, то можно их объединить в одно otherwise.

@alexeyr
Copy link
Owner

alexeyr commented Mar 29, 2024

Вдобавок ко всем замечаниям, нашёл, что у вас приличная часть скопирована из старой работы 21 года (из тех, что у меня сохранились). Ставлю 10, если это повторится, будет ниже. Повысить оценку исправлениями можно до 15.

@alexeyr
Copy link
Owner

alexeyr commented Apr 5, 2024

Повысил до 13.

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