Софтуерно Инженерство
Loading...
kosio197 avatar kosio197 104 Точки

[Java OOP Advanced Exam] Въпроси относно коментара към оценката.

Привет,

Пише, че не съм взел изпита със следния коментар:

"Липсват фундаментални знания от курса по ООП. Липсва Main метод. Структурата на кода е хаотична. Не е рефакториран framework-a. Не са оправени бъговете в него. Липсват Unit-Tests. Кодът е непреизползваем, тъй като липсва reflection. Липсват достатъчните анотации – класове, за да функционира програмата по задание."

Съдейки от горе написаното съм склонен да мисля, че става въпрос за някаква техническа грешка и оценката не е поставена върху кода който съм камитнал:

"Липсват фундаментални знания от курса по ООП" - Това е доста широко понятия. Аз го разбирам като сбъркани нейминг конвенции, лош избор на пакети и съответно лошо разпределяне на класовете в тях, липса на абстракция ... нещо такова, а това смятам, че като цяло е направено в моя код (може би има забележки, но чак да липсва ...)

"Липсва Main метод" - main метода е в bg.softuni.io.Main класа.

"Структурата на кода е хаотична." - това ми звучи пак като първия цитат в частта си с нейминг конвенциите и правилното именуване/разпределение по пакети. Отново, мисля, че дори и не докрай имплементиран кода не е хаотичен.

"Не е рефакториран framework-a. Не са оправени бъговете в него." - Наистина съм го използвал само отчати, но според условието това са 20 точки надолу и не би трябвало да е голям проблем.

"Липсват Unit-Tests" - Има юнит тест, който покрива нулевия тест от условието на задачата. Съгласен съм, че това е далеч от достатъчно, но не е и да липсва изцяло. (src/test/java package: bg.softuni.io.ZeroTest1.java)

" Кодът е непреизползваем, тъй като липсва reflection. " - Класовето в пакетите, които започват с bg.softuni.framework използват изцяло рефлекшън и напрактика могат да се използват на всякъде. Написано е по идея подобна на тази в BashSoft и тази от MarketPlace - по даден път сканират всичко там за анотации @Component и @Inject.

Освен това, класовете отговарящи за типа боклук се търсят с class.forName(package_name + typeName).

"Липсват достатъчните анотации – класове, за да функционира програмата по задание." - Кода не беше имплементиран докрай, макар, че не виждам причината за това да е в липсващи анотации, по - скоро не бях довършил имплементацията на някои класове (даже това го направих след изпита и в момента си дава максимум точки в judje, без да променям стария код - само го дописах и единственото което промених е, да не се сканира по път за файлове с анотации @Component, а да се подава списък с класове, тъй като judje не позволява да се чете директорията със сорса - иначе логиката за inject и autowire си е същата).

P.S. Целта ми не е да се заяждам, просто наистина не мога да свържа коментара с кода, който качих и моля да го погледнете пак.

Поздрави, Косьо

 

 

 

Тагове:
2
C# OOP Advanced
kaloyannikov avatar kaloyannikov 528 Точки

според мен това ,че си обединил Waste и DisposalStrategy в 1 клас Garbage - не е ок ( a и няма интерфейс)  и е излишно.

GarbageFactory-to в тоя си вариант не виждам особен смисъл от него - дали ще е 

Garbage garbage = new Garbage(type,strategy) или  = garbageFactory.createGarbage(type,strategy);

Дадена беше анотация @Disposable( която се слага върху други waste ) т.е трябва да имаш отделни класове за всеки 1 Waste (Burnable,Storable,Recyclable и etc.). При правилно именуване и добавяне на тази анотация върху тези класове после лесно без да се бърка никъде могат да се създават нови Waste types. Като и това фактори за Waste може да се направи с reflection. 

Също при създаването на Command  и метода getCommandInstance имаш изреждане на if ,else if, else if . В случая това е все 1 да имаш 1 switch. При положение че са именувани ок може да ползваш reflection и за тях , като според мен е добре и самия клас да завършва на Command , защото по този начин Status, TimeToRecycle, ProcessGarbage не съответсват на команда.

Самите стратегии също не са именувани ок ex -> BurnableWaste , така би трябвало да ти е именуван всеки Waste , а стратегията да е suffix - Strategy . Като погледне така кода човек и види BurnableWaste класа няма как да знае че това е стратегия, ако не отвори да види че имплементира DisposableStrategy.

За main метода да е в папка io , не ми се вижда ок също , по-скоро там трябва да са ти Reader и Writer и конкретни имплементации. Докато стартовата точка по-добре да е извън другите пакети за да се види ясно. Аз поне не бих очаквал да е в този пакет.

Това е моето мнение не казвам ,че съм прав.

Поздрави.

 

1
16/08/2016 11:23:10
kosio197 avatar kosio197 104 Точки

Привет,

@kaloyannikolov благодаря за мнението.

Повдигнатите теми в него ми звучат съвем логични, но не виждам връзка с коментара към оценката ми - ти по - скоро критикуваш имплементацията.

Сега ще ти обясня какво аз съм имал прeдвид: "според мен това ,че си обединил Waste и DisposalStrategy в 1 клас Garbage - не е ок ( a и няма интерфейс) и е излишно." - Защо смяташ, че не е ОК? Доколкото един Garbage has a DisposalStrategy мисля, че е достатъчно ОК. Иначе за интерфеса си прав че е хубаво да екстрактна.

"GarbageFactory-to в тоя си вариант не виждам особен смисъл от него - дали ще е ..." - В случая е така, оставил съм го като factory, за да може лесно да се добави вътре логика, като например това което казваш за анотацията. Ако бях ги правил с нови инстанции на всякъде добавянето на нова логика към създаването на обекта би била много по трудна.

"Като и това фактори за Waste може да се направи с reflection. " - То е направено с рефлекшън, намира си съответния клас за стратегията по име : return (GarbageDisposalStrategy) container.getInstance(Class.forName(typeName));, реално и да се добави нова стратегия, това фактори няма да се пипне.

"Също при създаването на Command и метода getCommandInstance имаш изреждане на if ,else if, else if . В случая това е все 1 да имаш 1 switch. При положение че са именувани ок може да ползваш reflection и за тях , като според мен е добре и самия клас да завършва на Command , защото по този начин Status, TimeToRecycle, ProcessGarbage не съответсват на команда."        - Съгласен, това би изглеждало по - добре с допълнителна анотация и рефлекшън. Тук наистина не ми стигна времето, тъй като има доста singleton класове и ми отне време да имплементирам контейнера и съответно autowire логиката.

"Самите стратегии също не са именувани ок ex -> BurnableWaste , така би трябвало да ти е именуван всеки Waste , а стратегията да е suffix - Strategy . Като погледне така кода човек и види BurnableWaste класа няма как да знае че това е стратегия, ако не отвори да види че имплементира DisposableStrategy."     

- Не смятам, че е нужно в името да има Strategy за да се разбере, че даденото нещо е стратегия, още повече, че Strategy го има в името на пакета. Не мисля и че твоето е лошо, просто е въпрос на личен избор.

Така например в по-голям проект от порядъка на 100 - 200 класа ако разчиташ на именоването на класовете си се загубил- и следователно е разумно да гледаш имената на пакетите когато търсиш конкретен клас.

"За main метода да е в папка io , не ми се вижда ок също , по-скоро там трябва да са ти Reader и Writer и конкретни имплементации. Докато стартовата точка по-добре да е извън другите пакети за да се види ясно. Аз поне не бих очаквал да е в този пакет."

- Ами напратика то си е input - output. Иначе не виждам смисъл да имплементирам Reader и Writer класове/интерфейси, тъй като то в самата java това си го има. Аз даже в самия main освен да викам вътрешен метод, който вече приема "InputStream stream, PrintStream printStream" друго не правя. То това всъщност ми и позволява да мога да си пиша unit tests със целите нулеви тестове.

"Това е моето мнение не казвам ,че съм прав."

- Ами то не мисля, че има някой абсолютно прав. Самите лектори не еднократно казаха, че много от конвенциите са въпрос на конкретна IT фирма и си има разлики - т.е. една задача има повече от едно вярно решение. Така, че според мен на това ниво вече наистина е важно да се коментират различни идеи за да можем по - добре да разбираме какво са мислили другите като са писали някакъв код, да имаме по - богат набор от идеи за имплементация на конкретен проблем и съответно бързо да можем да се впишем в IT фирмата, когато един ден започнем работа.

Поздрави, Косьо

0
StaVykoV avatar StaVykoV 169 Точки

Колегата Николов е напълно прав, но явно не се разбрахте нещо. Ще се опитам и аз да ти дам мнение:


"Липсват фундаментални знания от курса по ООП." - Чак да липсват знанията едва ли, но в случая са приложени, според мен, доста малко. Пакетите са наименувани доста неразбираемо, класовете и те имат неотговарящи на същността им имена. Както каза колегата, имаш клас "BurnableWaste". Аз това като го погледна веднага бих си помислил, че там ти е вид боклук, не стратегия. Също така липсва наследственост и полиморфизъм. Поне до колкото забелязах, имаш само един Waste. Тук съвсем спокойно може да имаше един абстрактен Waste и видове Waste които го наследяват. Като цяло кода е малко неразбираем и доста от класовете са написани сякаш просто за отбиване на номера - тоест не са имплементирани така, че да ти помогнат на теб самия, а само защото се изискват по условие.

"Липсва Main метод." - Main метода наистина трябва да е на най - видното възможно място. Иначе трябва да ти отворя всички пакети докато разбера къде се намира и чак тогава да мога да ти компилирам кода... Плюс това той няма много общо с IO освен това, че там декларираш обектите които ще ползват за тези функции. Реално в Main-а декларираш и доста други работи, така че не виждам логиката.

 

"Структурата на кода е хаотична." - виж първия коментар.

 

"Не е рефакториран framework-a. Не са оправени бъговете в него. " - не само, че не е отделен в самостоятелен пакет, но не мога въобще да го намеря. Има пакet Framework има пакет скелет и още някви други неща които ме водят на тая мисъл, но никъде не намерих фреймуърка отделен и така както беше. Струва ми се, че си променял имена на интерфейси. а може и вътре в тях, но това беше забранено. Цялата работа по framework-a беше да се фиксне, тъй като имаше 1-2 грешчици и да се вкарт проверките за листа. Възможно е аз да недоглеждам нещо, но е доста кашкаво.

 

"Липсват Unit-Tests." - това е ясно.

 

"Кодът е непреизползваем, тъй като липсва reflection."  - CommanInterpreter-a е с if-else, както каза и колегата. Това означава, че ако аз искам да ползвам твоя код и да добавя нова команда, трябва да ти пипам по CommandInterpretera. Това се има предвид под неизползваем. Виждам че си направил имплементация на inject-ване, но в случая тя ти върши почти никаква работа за преизползваемост на кода. Ако не ти е стигало времето, по - добре да беше направил един рефлешкън на 5 реда за командите и всички команди да се инстанцират с всички обекти които въобще се ползват. Така кода щеше да е преизползваем. Това за инджектването на точните обекти които са нужни за всяка команда, си е хем по - малко нужно, хем по - трудно и бавно за имплементиране.

 

"Липсват достатъчните анотации – класове, за да функционира програмата по задание." - по задание се изискваше да се ползва framework-a, а той изискваше да имаш анотации (които се ползваха за репозиторито) за всяка стратегия, които да имат мета анотацията от framework-a. При теб липсват тези анотации и въобще не ползваш репозиторито от framework-a.



Като цяло задачата наистина ми се струва доста хаотична и не са спазени дори основните задания. Не си ползвал framework-a, което е основна част от заданието. Много кофти именуване и подреждане на проекта - трудно се чете. Имай предвид, че за точките в джъджа може да се напише по много начини, та дори и всичко в един Main клас, но не това е целта. Аз лично не виждам за какво бих могъл да ти дам точки, ако бях проверяващ.

Не се сърди, не се опитвам да хейтя или тролвам. Дано критиката ми ти е полезна

Поздрави,
Блъдя

-1
16/08/2016 21:24:33
kosio197 avatar kosio197 104 Точки

"Не се сърди, не се опитвам да хейтя или тролвам. Дано критиката ми ти е полезна"

Не се сърдя даже напротив и откровенно казано всеки коментар ми е полезен.

Виждам че основноният дискутиран проблем е непреизползването на подаденият скелет което по условие е 20 точки и аз наивно реших да го пренебрегна.

Наблегнах на имплементация на фреимуърк - подобна на спринговската за инджектване и тя наистина стана много читава и 100% реюзъбал.

Също се омотах да използвам максимален брой дизаин патърни - пак част ото този курс.

Така оставен кода и на мен самия не ми харесва :)  така че изобщо не ти се сърдя за критиките просто не ми остана време да го рефакторна,ринеимна

и си зависна на работна версия.

Аз отворих темата заради коментара че ЛИПСВАТ меин метод , рефлекшън , юнит тест итн. което съгласи се е странно.

Поздрави , Косьо

 

 

 

0
kaloyannikov avatar kaloyannikov 528 Точки

Междудругото прекомерното ползване на патърни също не е ок , увеличава complexityto и става overdesign.

Иначе доста време си отделил на това да направиш този framework ,което не е лошо , но за времето което имахме си е трудно.

Аз на самия framework който ни дадоха + условието ги гледах доста време докато осъзная къде съм :D .

1
kosio197 avatar kosio197 104 Точки

"Междудругото прекомерното ползване на патърни също не е ок , увеличава complexityto и става overdesign"

Е най-накрая да го каже и някои друг.

Много одавна споделих това мнение но срешнах сериозен отпор и от тогава на изпит се опитвам да "натреса " общо взето всичко преподавано.

Поздрави, Косьо

0