[conceptueel] Encapsulation / Separation of concerns

[Carharttguy]

Beste mede-Tweakers

Ik heb het sinds jaar en dag soms moeilijk om code conceptueel juist uit te denken. ik heb het gevoel dat ik altijd een compromis moet maken tussen twee slechte keuzes, en dat terwijl ik geen code schrijf die baanbrekend nieuw is. Ik loop nu tegen een conceptueel probleem aan, waarbij ik voel dat mijn Encapsulation en Seperatino of concerns tegen elkaar botsen. Even een paar aannames, het zou kunnen dat ik al fout zijn bij deze premises (waarvan ik denk dat ik ze geleerd heb op de hogeschool, een tijd terug):

-Classes moeten zoveel mogelijk self-contained zijn, het is niet de bedoeling dat je 30 classes in een project moet importeren om er eentje van te gebruiken.

-Classes moeten niet meer doen dan je ervan verwacht. Niet meer, niet minder: Seperation of concerns.

Nu een praktijkvoorbeeld dat ik gisteren tegenkwam. Ik heb een database met daarin een 'users' tabel, waar allemaal users inzitten. In m'n OOP taal heb ik dus een database class, om te verbinden met de database en bewerkingen te doen en een User class.

Maar nu, stel ik wil een manipulatie doen op een User instantie. Bijvoorbeeld een nieuw wachtwoord geven aan de user. Een nieuw wachtwoord geven houdt in: Het aanmaken van een nieuwe random salt, wachtwoord en salt hashen, de waarde wegschrijven naar de database én de hash aan het wachtwoord member van de user instantie toewijzen (dat laatste is niet echt nodig, maar het komt goed uit om mijn 'probleem' te schetsen)

Nu kan ik (denk ik) twee kanten op:

Optie 1) Ik maak een method op de User class 'User.SetNewPassword(string)' en de user een private database instantie geven zodat deze verbinding maakt met de database en zelf de queries e.d. afhandeld.

Problemen:
1) Seperation of concerns: Een User class moet in principe alleen een user representeren, een user weet niet wat een database is, en zou ook geen wachtwoorden van databases moeten bevatten of queries uitvoeren.

2) Performance: als er 1000 gebruikers zijn, zijn er potentieel 1000 open connecties naar de databank, wat eigenlijk overkill is.

Optie 2) Ik geef de databaseclass een bepaalde awareness voor de User class, zodat deze de user class gebruikt en manipuleert: 'db.SetUserNewPassword(userinstantie, string)'. In dit geval zal de database zelf de queries uitvoeren en de 'User.PasswordHash' veranderen.

Problemen:
1) De database class en de User class zijn onlosmakelijk met elkaar verbonden. Ik dat de databaseclass dus niet hergebruiken zonder de user class.

2) De database class veranderd de User.PasswordHash waarde, dus moet deze public settable zijn, wat betekend dat elke andere class deze ook kan aanpassen, dit kan dus de integriteit van mijn data betekenen, de PasswordHash kan namelijk anders zijn in mijn instantie dan in de database.

Hebben jullie ideeën? Wat is een goede methode, zijn er andere manieren (waarschijnlijk) die logischer zijn en ik niet zie of vergeet?

Hartelijk dank voor jullie input.

[Hmail]

Ik ben niet geweldig thuis in design patterns, maar volgens mij is een oplossing hiervoor het Repository Pattern.
Symfony lost dit op met EntityManager waarmee je een Repository kan krijgen die afweet van een User.

Het probleem van teveel classes lijkt me niet echt iets om me druk over te maken.

[Adimeuz]

Je geeft eigenlijk al aan dat optie 1 geen optie is omdat je "User" opeens verantwoordelijkheden gaat geven die het helemaal niet mag hebben. Daar ben ik het mee eens.

Daarnaast geef je als probleem aan dat optie 2 tight coupling veroorzaakt doordat de databaselaag/klasse "User" kent. De gangbare methode om tightly coupled --> loosly coupled te maken is door je klassen afhankelijk te maken van een abstractie, en niet van een implementatie, door bijvoorbeeld een interface te maken voor het userobject (Misschien zegt de term 'inversion of control' je iets, zo niet, is er aardig wat leesvoer over te vinden).

Het tweede probleem dat je noemt bij optie 2 zie ik niet helemaal. De database-klasse heeft één verantwoordelijkheid: database I/O. Het schrijft een waarde naar, of leest een waarde van, de database. Dat de hash aangepast is zal de databaseklasse een zorg zijn, die aanpassing doe je maar op een plaats waar de verantwoordelijkheid voor het aanpassen van userdata kan liggen (business-layer, of misschien wel gewoon de GUI?)

[ Voor 34% gewijzigd door Adimeuz op 18-09-2017 16:51 ]

[farlane]

"Add another layer of indirection", waarbij de user de database niet hoeft te kennen en de database de user niet.

[The Eagle]

Kort door de bocht loop je als developer dus structureel tegen architectuurvragen aan. Wat is goed, wat is een guideline, wat is de standaard?
Welnu, dat kan overal anders zijn. Verdiep je eens in architectuurframeworks (togaf, nora etc) en ik denk dat je dan zult zien dat er geen enkele oplossing de beste is, maar er wel een mogelijkheid is om bepaalde architectuurbeslissingen te nemen en je daar verbolgens aan te houden. En soms kost dat dan meer werk en kan het makkelijker, soms ook niet. Je bent dan iig consistent in wat je voor een bepaald stuk software oplevert

[Morax]

Naar mijn idee is het wijzigen van een wachtwoord een verantwoordelijkheid van de User class: Deze hoort te weten hoe het wachtwoord gehashed moet worden en uiteindelijk ook hoe je kan controleren of een wachtwoord correct is.
Wanneer je in theorie een tweede User class erbij krijgt (User_Admin bijvoorbeeld) waarbij je de wachtwoorden anders wilt beveiligen, dan hoort die logica dus in die class te liggen.

Wat betreft de koppeling tussen User en DB, dat ligt wat lastiger. Als eerste heb je een abstractielaag nodig die de achterliggende database onafhankelijk maakt. Meestal in de vorm van bijvoorbeeld een ActiveRecord implementatie.
Vervolgens heb je een class nodig die alle verbindingen met een database beheert, ik noem het even de Connection class. Daar zou ik dan alle benodigde verbindingen inladen met een identifier, waarbij je de standaardverbinding inlaad met de identifier "default".
Verder neem ik even aan dat je User class extend van een bepaalde Base class om alle save, update, delete functies en dergelijke voor je te implementeren (die wil je niet opnieuw schrijven bij elke nieuwe class die iets op moet slaan in jde DB). Deze base class geef je een standaard public function getConnectionIdentifier() die standaard "default" teruggeeft. Elke nieuwe class die iets opslaat slaat nu standaard zijn data op in de "default" connectie. Een subclass heeft nu echter de mogelijkheid om de functie getConnectionIdentifier() te overriden om zijn data in een andere databaseconnectie op te slaan.

Nu maakt het voor de user class niet meer uit welke database gebruikt wordt en waar deze staat.
Indien je een ActiveRecord of andere abstractielaag gebruikt maakt het type database niet meer uit.
Elke class die iets opslaat kan nu zelf aangeven in welke datastore hij terecht moet komen. Meerdere classes kunnen dus in meerdere datastores iets opslaan (en dat kan door elkaar heen gebruikt worden).

Just my 2 cents...

[ Voor 11% gewijzigd door Morax op 19-09-2017 12:03 ]

[Stoelpoot]

Carharttguy schreef op maandag 18 september 2017 @ 15:34:
Ik heb een database met daarin een 'users' tabel, waar allemaal users inzitten. In m'n OOP taal heb ik dus een database class, om te verbinden met de database en bewerkingen te doen en een User class.

Hier gaat het fout met je seperation of concerns. De database hoort namelijk geen bewerkingen te doen op de User class. De database slaat de user class op zodra je aangeeft dat deze bewerkt is en opgeslagen moet worden. De database in dit geval is een heel ambiguous iets wat niets anders kan dan data wegschrijven en ophalen. Dit kan namelijk een slimme server als SQL zijn, maar ook een tekstbestand, een binair bestand, of een administratief medewerker die een mailtje krijgt 'schrijf dit op op het papiertje van gebruiker 123'.

Nu kan je dus een hele andere kant op. De user kan best z'n eigen wachtwoord bepalen, door dus 'User.SetPassword(str)' aan te roepen. Dit slaat de user dan op in z'n eigen class, waarna de database ervoor zorgt dat dit password ook wordt weggeschreven. Voor de database is het wachtwoord maar een stuk tekst zonder betekenis, net als de username van de user.

Maar je kan ook je login-systeem het wachtwoord laten bepalen en dit dan toewijzen aan de user.password. Het login-systeem is dan verantwoordelijk voor het correct behandelen van het wachtwoord. Dit is logischer dan het door de user te laten doen, omdat het login-systeem (waarschijnlijk) ook controleert of wachtwoorden gelijk zijn bij login. De user-class wordt dan een heel gewoon object wat geen enkel besef heeft dat zijn wachtwoord beveiligd is. Dat maakt ook niet uit, het gaat er enkel dat dat bij een user een wachtwoord is ingevuld. Dit is een patroon wat je vaak ziet, en ik denk dat dit dan ook een nette aanpak is.

[Morax]

Stoelpoot schreef op dinsdag 19 september 2017 @ 12:14:
[...]


Hier gaat het fout met je seperation of concerns. De database hoort namelijk geen bewerkingen te doen op de User class. De database slaat de user class op zodra je aangeeft dat deze bewerkt is en opgeslagen moet worden. De database in dit geval is een heel ambiguous iets wat niets anders kan dan data wegschrijven en ophalen. Dit kan namelijk een slimme server als SQL zijn, maar ook een tekstbestand, een binair bestand, of een administratief medewerker die een mailtje krijgt 'schrijf dit op op het papiertje van gebruiker 123'.

Nu kan je dus een hele andere kant op. De user kan best z'n eigen wachtwoord bepalen, door dus 'User.SetPassword(str)' aan te roepen. Dit slaat de user dan op in z'n eigen class, waarna de database ervoor zorgt dat dit password ook wordt weggeschreven. Voor de database is het wachtwoord maar een stuk tekst zonder betekenis, net als de username van de user.

Maar je kan ook je login-systeem het wachtwoord laten bepalen en dit dan toewijzen aan de user.password. Het login-systeem is dan verantwoordelijk voor het correct behandelen van het wachtwoord. Dit is logischer dan het door de user te laten doen, omdat het login-systeem (waarschijnlijk) ook controleert of wachtwoorden gelijk zijn bij login. De user-class wordt dan een heel gewoon object wat geen enkel besef heeft dat zijn wachtwoord beveiligd is. Dat maakt ook niet uit, het gaat er enkel dat dat bij een user een wachtwoord is ingevuld. Dit is een patroon wat je vaak ziet, en ik denk dat dit dan ook een nette aanpak is.

Het login systeem wil je echter hier niet verantwoordelijk voor maken. Wie weet wil je op een andere plek ook een password check doen? Of misschien wil je via een adminpagina een nieuw wachtwoord laten genereren. Dat gaat niet als de logica in de login verwerkt zit!

Wat je misschien wel wil is de flow van een login vastleggen in een interface, zodat je login systeem niet meer afhankelijk is van de User class, maar je ook andere classes een login kan laten doen

[Stoelpoot]

Morax schreef op dinsdag 19 september 2017 @ 12:21:
[...]


Het login systeem wil je echter hier niet verantwoordelijk voor maken. Wie weet wil je op een andere plek ook een password check doen? Of misschien wil je via een adminpagina een nieuw wachtwoord laten genereren. Dat gaat niet als de logica in de login verwerkt zit!

Wat je misschien wel wil is de flow van een login vastleggen in een interface, zodat je login systeem niet meer afhankelijk is van de User class, maar je ook andere classes een login kan laten doen

Het gaat niet over de login-pagina, maar het login-systeem. Een betere term zou eigenlijk UserManager zijn, wat dus een klasse zou zijn die de gegevens voor login, registratie, en sessies van users beheerd. Als je er op die manier naar kijkt is het logisch dat deze op elk moment beschikbaar is. Je zou aan een UserManager ook kunnen vragen wie er op dat moment is ingelogd, bijvoorbeeld.

[farlane]

Stoelpoot schreef op dinsdag 19 september 2017 @ 12:25:
[...]
... UserManager...

"Add another layer of indirection"

[Adimeuz]

Stoelpoot schreef op dinsdag 19 september 2017 @ 12:25:
[...]
Het gaat niet over de login-pagina, maar het login-systeem. Een betere term zou eigenlijk UserManager zijn, wat dus een klasse zou zijn die de gegevens voor login, registratie, en sessies van users beheerd. Als je er op die manier naar kijkt is het logisch dat deze op elk moment beschikbaar is. Je zou aan een UserManager ook kunnen vragen wie er op dat moment is ingelogd, bijvoorbeeld.

Oneens, als ik zo vrij mag zijn. UserManager breekt het SRP, alleen de naam *Manager suggereert al dat het SRP breekt, en dat blijkt nogal waar te zijn wanneer die klasse verantwoordelijk is voor registratie, login, sessies, etc.

[farlane]

Adimeuz schreef op dinsdag 19 september 2017 @ 15:42:
[...]
Oneens, als ik zo vrij mag zijn. UserManager breekt het SRP, alleen de naam *Manager suggereert al dat het SRP breekt, en dat blijkt nogal waar te zijn wanneer die klasse verantwoordelijk is voor registratie, login, sessies, etc.

Als je die verantwoordelijkheden splitst naar verschillende *Managers dan?

[Agshlee]

farlane schreef op dinsdag 19 september 2017 @ 16:02:
[...]

Als je die verantwoordelijkheden splitst naar verschillende *Managers dan?

Wat mij betreft zou dat geen verschil maken. Immers, als *Manager als naam het beste bij een class past, dan doet deze class by definition meerdere zaken (en voldoet daarmee dus niet aan SRP).

Of je alles ten alle tijde aan het SRP kan laten voldoen is natuurlijk een andere discussie.

[farlane]

Agshlee schreef op dinsdag 19 september 2017 @ 16:08:
[...]
Wat mij betreft zou dat geen verschil maken. Immers, als *Manager als naam het beste bij een class past, dan doet deze class by definition meerdere zaken (en voldoet daarmee dus niet aan SRP).

Of je alles ten alle tijde aan het SRP kan laten voldoen is natuurlijk een andere discussie.

En al het *echt* maar 1 ding doet? Of is dan de naam verkeerd?

[Agshlee]

farlane schreef op dinsdag 19 september 2017 @ 16:28:
[...]

En al het *echt* maar 1 ding doet? Of is dan de naam verkeerd?

In dat geval zou ik de naam veranderen. Stel, de class past alleen een wachtwoord aan, dan zou deze class ook PasswordChanger (o.i.d.) kunnen heten.

Los hiervan denk ik dat de TS ook eens naar de repository pattern kan kijken.
- User: simpele class met properties
- UserRepository: ophalen en wegschrijven van user informatie;
- Database.

Eventueel kan er nog gebruik gemaakt worden van een Service, welke gebruik maakt van de repository, welke ervoor zorgt dat de informatie in de database wordt opgeslagen.

[Beyond]

Wij doen het zo:

Wij ontkoppelen de UI van de backend. Door middel van REST praten wij tegen onze backend. De backend ziet er zo uit:

Controller / Service (1) > Repository (2) > Database

Business Logic in 1, dus het veranderen van het wachtwoord op het user object. Deze geeft de gewijzigde user door aan 2.

Zowel 1 als 2 maken gebruik van interfaces die wij kunnen mocken in de unit tests. Zodat elke laag apart is te testen.

Wij gebruiken Java / Spring / Hibernate / SQL server voor de backend.

Angular voor de frontend en dat wordt op den duur ook nog een app.

[ Voor 3% gewijzigd door Beyond op 19-09-2017 21:04 ]

[mulder]

Even voor de duidelijkheid:

Jij hebt het over SoC: Wikipedia: Separation of concerns

Maar het klinkt meer als SRP: Wikipedia: Single responsibility principle

En ik hoor een beetje Unit Of Work: https://martinfowler.com/eaaCatalog/unitOfWork.html

[H!GHGuY]

farlane schreef op dinsdag 19 september 2017 @ 16:28:
[...]

En al het *echt* maar 1 ding doet? Of is dan de naam verkeerd?

Als iemand afkomt met een "Manager" class dan stel ik meteen een pertinente vraag:
Is die manager echt alleen een collectie van subordinates waarheen de manager werk off-load? Want, net zoals in het echte bedrijf "werkt" een manager niet zelf, hij deelt enkel taken uit en speelt hoogstens als een soort liaison tussen verschillende afdelingen en ondergeschikten.

Hoe grappig het ook klink, een manager doet zelf geen werk. Hij dispatcht werk naar de leden in zijn team.