Skip to content

fgfg#1

Open
mak271 wants to merge 4 commits into
masterfrom
Other
Open

fgfg#1
mak271 wants to merge 4 commits into
masterfrom
Other

Conversation

@mak271
Copy link
Copy Markdown
Owner

@mak271 mak271 commented Feb 1, 2021

Test

mak271 and others added 2 commits February 1, 2021 12:55
Comment thread src/Devices/Device/Combine.kt Outdated
@@ -0,0 +1,26 @@
package Devices.Device
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Случайно продублировал три класса. Из кода их уже удалил.

Copy link
Copy Markdown

@VladimirBelyakof VladimirBelyakof left a comment

Choose a reason for hiding this comment

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

Ок. Прокоментировал все.
Почитай подумай, как можно убрать существующие недостатки.
Еще по схеме БД вроде не было информации в реквесте вроде.
Если будут вопросы пиши, но я только направить могу или какие-то отдельные сложные моменты разяъснить. В основном старайся сам информацию найти

Comment thread .idea/vcs.xml
<component name="VcsDirectoryMappings">
<mapping directory="$PROJECT_DIR$" vcs="Git" />
</component>
</project> No newline at end of file
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Да в ignore ты его добавил. Но не до конца понял как это работает. Сейчас ты все еще предлагаешь этот файл добавить в проект. Иначе бы он тут не отображался. И я не смог бы написать тут комментарий. Нужно сделать так чтобы этого фала не было в реквесте

Comment thread src/Cabinets/Cab.kt Outdated
import Interfaces.Observer
import Interfaces.Subject

abstract class Cab(office: Subject) : Observer {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

В ООП должны быть четкие абстракции. Например Корова являеется животным.
В тввоем случае Кабинет является Наблюдателем? Помещением может быть, видом комнаты возможно, но при чем тут наблюдатель?

Comment thread src/Cabinets/Cabinet2.kt Outdated

import Interfaces.Subject

class Cabinet2(office: Subject) : Cab(office) No newline at end of file
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Зачем тебе 3 одинаковых класса?
Каждый класс должен асоциороваться с объектом реального мира. Такое чувство, что ты хочешь создать просто новый объект класса.
Давай дам пример. Если я создаю иерархию классов на примере живтоных, то я бы создал базовый класс животное и несколько наследников, напрмер собака которая имеет родословную и корову, которая умеет давать молоко.
Если бы я хотел создать двух коров это были бы два объекта данного класса Cow("Буренка") и Cow("Мэри")
Подумай почитай в каких случаях создается новый класс а в каких я могу просто вынести разницу в аргументы контруктора.

Comment thread src/Cabinets/Cabinet3.kt Outdated

import Interfaces.Subject

class Cabinet3(office: Subject) : Cab(office) No newline at end of file
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Кабинет не должен принимать родителя в контруктор.
Классы нужно делать настолько независимыми от других, насколько это возможно.
Вдруг ты захочешь сделать кабинет у себя дома. Как быть все переписывать? Можно конечно извратиться, но в целом это неправильно. Кабинет это составная часть оффиса но не наоборот

Comment thread src/Employers/Human.kt Outdated

init {
emp.addEmp(this)
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

То же самое. Зачем человеку знать о каком то внешнем хранилище где он может находиться. А если потребуется завтра создать человека который не нахоидтся ни в каком списке. Читай понимай про абстракции. Имя, Фамилия являются частью человека. Какой-то список сотрудников абсолютно не нужен и не понятен для существования человека.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

И еще нарушение принцыпов наследования. Корова всегда является животным, собака всегда является животным. легушка всегда является животным. Для них мы выделяем общую абстракцию "Животное".

Разве любой человек является работником? Совсем нет. (или что значт Works полагаю ты имел ввиду Worker или Employer)

Comment thread src/Interfaces/Works.kt Outdated

interface Works {
fun upd(name: String, surname: String)
} No newline at end of file
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Works не понятно. Интерфрейс/класс как провало это существительное в единственнном числе. Ты хотел сказать Worker или Work? В любом случае в данной абстракции сложно представить назначение метода upd. что он должен делать?

Comment thread src/Office.kt Outdated

class Office(var cabinets: ArrayList<Observer> = ArrayList(), var employers: ArrayList<Works> = ArrayList()): Subject, Employers {

private var number: Int = 0
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Немного не понял. В задании было что кабинет имеют номрера. Номер оффиса, это намерено сделано или ошибка?

Comment thread src/Office.kt
val cab: Observer = cabinets[i]
number++
cab.update(number)
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Не понял что тут просиходит. Пробегаем по кабинетам и увеличиваем им номер на 1?

Comment thread src/Office.kt Outdated

fun showAllCabinets() {
notifyCabinets()
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Так вывыести все кабинеты или уведомить все кабинеты о чем-то? ОПять же непонятяно о чем уведомить

Comment thread src/Office.kt
val emp: Works = employers[i]
emp.upd("", "")
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

То же самое что с оффисами

Comment thread .gitignore
@@ -1,2 +1,3 @@
# Project exclude paths
.idea/vcs.xml
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Добавил не в тот gitignore.

Comment thread src/Cabinet.kt Outdated

class Cabinet(var number: Int) : Display {

private var device: String = ""
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Почему в кабинете находится только название устройства? У нас в кабинете настоящий принтер, а не только бирка от него

@VladimirBelyakof
Copy link
Copy Markdown

И еще, где грамотно раксставленные модификтаоры доступа?

Попрошу тебя отписаться в комментариях где и какой принцып ООП Ты применил. И зачем.
Наследование, инкапсуляция, полиморфизм - все обязательно должно быть продемострировано

Comment thread src/Cabinet.kt Outdated

fun addEmployer(employer: Employer)
{
employers.add(employer).toString()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

.toString() Это зачем?

Comment thread src/Display.kt Outdated
interface Display {
fun displayDev()
fun displayEmp()
} No newline at end of file
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

  1. мне не понятен этот интерфрейс. Он называется Display. Ок, достаточно общая абстракция. Хотя конечно сказать что Кабинет является Экраном.. странно. Ну допустим.
  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