Skip to content

Lab 02#46

Open
lamposhka wants to merge 5 commits into
Rrenkens:mainfrom
lamposhka:lab-02
Open

Lab 02#46
lamposhka wants to merge 5 commits into
Rrenkens:mainfrom
lamposhka:lab-02

Conversation

@lamposhka

Copy link
Copy Markdown

No description provided.

"bread" : 50,
"butter" : 50
},
"hobos": 1,

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.

По условию В доках обитает hobos > 2 бродяг.

throw new IllegalArgumentException("Wrong arguments provided.");
}
String path = args[0];
initializeController(path);

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.

Кмк, лучше было бы сделать так controller = getController().

Comment on lines +28 to +30
public static Controller getInstance() {
return instance;
}

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 Dock implements Runnable {
int unloadingSpeed;
ConcurrentHashMap<String, Integer> storageTaken = new ConcurrentHashMap<>();

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.

А зачем ConcurrentHashMap если все равно все выполняется под синхронизацией.

Comment on lines +33 to +35
for (var i : storageTaken.keySet()) {
System.out.println(i + storageTaken.get(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.

А это зачем?

Comment on lines +49 to +51
for (var i : storageTaken.keySet()) {
System.out.println(i + storageTaken.get(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.

А это зачем?

}

public void setCargo(String cargo) {
synchronized (this) {

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.

лучше тогда метод пометить synchronized. Но в текущем коде, все это происходит в однмо потоке, поэтому тут это не нужно как и ниже.

if (cargo == null) {
continue;
}
synchronized (Controller.getInstance()) {

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.

А зачем это тут? Контроллер нигде не изменяется

Comment on lines +69 to +70
ingredientsStolen.put(cargoToSteal, ingredientsStolen.get(cargoToSteal) + 1);
logger.log(Level.INFO, ingredientsNeeded + " " + ingredientsStolen);

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.

Почему? Тут же надо убедиться, что он своровал объект.

@Rrenkens

Rrenkens commented Dec 28, 2023

Copy link
Copy Markdown
Owner

Основная оценка:

  • -0.75 за лишние синхронизации, и бесполезный ConcurrentHashMap.
  • -1 за то, что сперва прибавляется ворованное, а потмо уже воруется.

Допы:

  • +2 за CV.

В итоге:
Основная оценка – 8.25.
Допы – 2.

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