Skip to content

Lessons#85

Open
Kegeke wants to merge 48 commits intoKFalcon2022:for-prfrom
Kegeke:for-pr
Open

Lessons#85
Kegeke wants to merge 48 commits intoKFalcon2022:for-prfrom
Kegeke:for-pr

Conversation

@Kegeke
Copy link
Copy Markdown

@Kegeke Kegeke commented May 13, 2024

No description provided.

Kegeke added 3 commits May 13, 2024 10:19
Сделал класс Car immutable
if(f.getFileName().equals(file)) {
System.out.println(f);
return;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

лишняя пустая строка

@@ -0,0 +1,5 @@
package com.walking.lesson20_exceptions.task1_catchException.model;

public enum TypeInformation {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

InformationType

seachFile(file, files);
}

private static void seachFile (String file, File [] files) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Формально задача решена. Но я рекомендую уделять больше внимания декомпозиции и проектировать решения с использованием классов. Это будет полезно вне зависимости от изучаемой темы

Так, со временем станет понятно, что void-методы - не всегда хорошее решение. И метод с названием seachFile() по своей логике должен возвращать найденный объект типа File:)

В целом, советую иногда фантазировать - насколько тяжело будет переиспользовать решение, если изменится формат ввода (скажем, вместо консоли - обработчик нажатия на кнопку в графическом интерфейсе) или вывода (скажем, не в консоль, а в файл)

@Kegeke
Copy link
Copy Markdown
Author

Kegeke commented May 20, 2024

lesson26 task2 неправильное решение

@Kegeke Kegeke requested a review from KFalcon2022 May 21, 2024 05:10
@@ -0,0 +1,7 @@
package com.walking.lesson28_generics1.task4.exception;

public class ElementDoesNotExist extends RuntimeException{
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

пробел перед {

@@ -0,0 +1,7 @@
package com.walking.lesson28_generics1.task4.exception;

public class ElementNotFoundException extends RuntimeException{
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

пробел

import java.util.Objects;

public class Stack<T> {
private final Object[] stack;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Owner

@KFalcon2022 KFalcon2022 May 22, 2024

Choose a reason for hiding this comment

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

и не позволяет в полной мере поработать именно с дженериками - при извлечении все сводится к касту


public void push(T item) {
if (tos == stack.length - 1) {
System.out.println("Стек полон");
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

у стека особо нет ограничений по длине:) логичнее после этой строки вставить return (или заменить принт в консоль на эксепшн, что более оправдано) и код ниже писать без else

public Object pop() {
if (tos < 0) {
throw new ElementDoesNotExist("Елемент не существует.");
} else {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

else избыточен. Если зашло в иф с return/throw, в следующие блоки в любом случае не зайдет

}
}

public Object getValue(T value) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

не хватает каста

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

и метод логичнее назвать аля find


public class Stack<T> {
private final Object[] stack;
private int tos;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

название должно быть осмысленно


@Override
public boolean equals(Object o) {
if (this == o) return true;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

потерял {}

private static TreeMap<Integer, Car> initCars() {
TreeMap<Integer, Car> cars = new TreeMap<>();

cars.put(1, new Car("RRR-111-XX", 2021, "red", true));
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

кажется, ты не разобрался, зачем нужен ключ у Map

public class CarService {
private final TreeMap<Integer, Car> carService;

public CarService(TreeMap<Integer, Car> carService) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

параметр - явно не carService

public String findCar(Car car) {
for (Car c : carService.values()) {
if (c.equals(car)) {
return c.toString();
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

почему строку-то возвращаем?) с ней явно не получится работать дальше как с машиной


public class Philosopher extends Thread {
private final String name;
private final Semaphore semaphore;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Некорректно с точки зрения дизайна - семафор, выступающий посредником, не является ни частью философа, как сущности, ни частью логики философа. Это не значит, что так делать в принципе нельзя, но риск проблем при таком решении достаточно высок при любом дальнейшем развитии решения

public static void main(String[] args) {
Semaphore semaphore = new Semaphore(5);

for (int i = 1; i < 6; i++) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

традиционно циклы нумеруются от 0, если нет явной необходимости в ином. Учитывая, что здесь число обозначает порядковый номер философа - подход имеет смысл, но я бы советовал в таком случае ограничить цикл по <= 5 - в таком случае будет понятнее, что это за числа и к чему они привязаны

private final Semaphore semaphore;

private final int id;
private int num = 0;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

0 - значение по умолчанию для числового поля

private int num = 0;

public Philosopher(String name, Semaphore semaphore, int id) {
this.name = name;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

избыточное поле. Могло бы иметь смысл, не будет у философа числового айди

semaphore.acquire(2);
System.out.printf("%s %d садится за стол.\n", name, id);

takeLeftFork();
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

наверно, основной недостаток твоего решения - ты никаким образом не формулируешь сущность "вилка". Т.е. это некая абстракция, которая не имеет воплощения. Но при этом вилка - это четко опрделенный разделяемый ресурс.
Из-за этого допущения, как вижу, вполне возможна ситуация, в которой у тебя будут есть два соседних философа одновременно, несмотря на то, что у них на двоих в доступе лишь 3 вилки из требуемых 4: слева от философа1, справа от философа1(==слева от философа2) и справа от философа2

semaphore.release(2);

sleep(500);
System.out.printf("%s %d размышляет.\n", name, id);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

чутка нелогично - ты выводишь это в консоль к моменту окончания размышлений:)

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