drashland / rhum

A test double library
https://drash.land/rhum
MIT License
91 stars 6 forks source link

Issue #19 non-public functionality #84

Closed crookse closed 3 years ago

crookse commented 3 years ago

Fixes #19

Description

Guergeiro commented 3 years ago

I'm against this change. There's a reason why private/public/protected signatures exist. Making this kind of change will violate encapsulation in OOP. If something needs to be accessed it should be either by using Getters/Setters or making the attributes public. Using the latter is the "lazy" way, btw.

I would find the specific points trying to access and provide Getters for them.

crookse commented 3 years ago

@Guergeiro, so you don't see a use case to test non public methods?

ebebbington commented 3 years ago

I would have to disagree with @Guergeiro, it's not as if private/protected methods are accessible all the time - you'd have to directly explicitly use rhum to do this - plus, we can't count on other API's having getters and setters.

Guergeiro commented 3 years ago

@Guergeiro, so you don't see a use case to test non public methods?

Testing private methods should be done with Reflection and not with a method that changes the class itself.

crookse commented 3 years ago

so should we mimic a reflection API?

Guergeiro commented 3 years ago

I would have to disagree with @Guergeiro, it's not as if private/protected methods are accessible all the time - you'd have to directly explicitly use rhum to do this - plus, we can't count on other API's having getters and setters.

That method is an API for the end user... That's the issue. An API shouldn't expose everything to the user, specially something which is related to the framework internal behaviour and not useful in anyway besides framework maintainers.

ebebbington commented 3 years ago

Yeah i guess i see your point - i mean we take that approach with drashland modules already (or any code)

crookse commented 3 years ago

Breno makes a good point. we will add documentation on how to reflect classes to test non-public members, but we will not be exposing an API to do it for users.