#202 Registrere meldingsdetaljer for innkommende meldinger #135
Conversation
kfh
left a comment
kfh
left a comment
There was a problem hiding this comment.
Bare litt småtteri fra meg ellers ser jo dette bra ut 👍
ebms-provider/src/main/kotlin/no/nav/emottak/ebms/util/EventRegistration.kt
Show resolved
Hide resolved
| } | ||
| } | ||
|
|
||
| class EventRegistrationServiceFake : EventRegistrationService { |
There was a problem hiding this comment.
Litt nitpicky, men navngivningen bør være FakeEventRegistrationService.
Her kan du også implementere dette som en funksjon som beskrevet ovenfor. Du får da:
fakeEventRegistrationService( ... )
There was a problem hiding this comment.
Hvordan er FakeEventRegistrationService bedre enn EventRegistrationServiceFake ?
There was a problem hiding this comment.
MyService, FooService, BarService er jo typisk et pattern vi vil følge.
There was a problem hiding this comment.
Hvorfor har vi slike viljer?
Det siste gang jeg spurte, så hadde teamet vårt ingen kodestandard.
There was a problem hiding this comment.
Hva mener du ? Dette er helt STANDARD navngivning.
There was a problem hiding this comment.
Hver gang når jeg prøvde å bruke noe standard i eMottak, så fikk jeg avslag og flere klager fordi
vi eksperimenterer og ingenting er skrevet på stein
Jeg prøver å forstå hvorfor det ble plutselig ikke lov til å eksperimentere med navngivelse?
There was a problem hiding this comment.
Kort fortalt så ser det litt rart ut. Hvis det hadde vært konsistent, dvs ALLE service-implementasjoner hadde blitt post-fixet med implementasjon, ala: MyServiceFoo, MyServiceBar, MyServiceFake hadde det hvertfall vært gjennomført og konsistent.
There was a problem hiding this comment.
Og det er akkurat slik akkurat nå.
EventRegistrationServiceImpl og EventRegistrationServiceFake er de eneste service-implementasjoner i hele prosjektet (kanskje i hele eMottak også) resten av services er vanlige klasser uten den interface overengineering som bare forstyrer smidighet.
There was a problem hiding this comment.
Det ser fortsatt rart ut og EventRegistrationServiceFake leser ikke noe godt, imho. Det er hvertfall ukonvensjonell måte å navngi på. Jeg skal ikke blokke PR'en pga av dette, men min personlige preferanse er at du hadde implementert interfacet via en funksjon slik jeg foreslo. Da hadde du også sluppet unna med denne redundante bruken av Impl i klassenavnet og brukt elegante features i Kotlin. For meg føles dette veldig Java'ish ut.
There was a problem hiding this comment.
Takk for at du er konstruktiv! 🙌
OleksandrChmyrNAV
deleted the
#202-registrere-meldingsdetaljer-for-innkommende-meldinger
branch
EventRegistrationService.@OptIn(ExperimentalUuidApi::class)merknad fra klasser.