Replace get/setSex with get/setGender #95

heiglandreas · 2019-09-15T16:49:15Z

As explained in #94 it's a good idea to use gender instead of sex.

To not break BC the get/setSex are still available but are marked as deprecated. The current functionality works as before. In addition to that it is possible to set other as gender and the default now is to return unknown instead of NULL.

As explained in #94 it's a good idea to use gender instead of sex. To not break BC the get/setSex are still available but are marked as deprecated. The current functionality works as before. In addition to that it is possible to set `other` as gender and the default now is to return `unknown` instead of NULL.

ADmad · 2019-09-15T16:50:12Z

src/Common/Entity/User.php

     */
-    protected $sex;
+    protected $gender = GENDER_UNKNOWN;


Shouldn't it be self::GENDER_UNKNOWN?

You got me there! Sure!

ADmad · 2019-09-15T16:51:39Z

src/Common/Entity/User.php

-        if ($sex !== self::SEX_MALE && $sex !== self::SEX_FEMALE) {
-            throw new \InvalidArgumentException('Argument $sex is not valid');
+        $genders = [
+            GENDER_OTHER,


All constants should have static:: prefix.

ADmad · 2019-09-15T16:52:45Z

src/Common/Entity/User.php

+            GENDER_MALE,
+            GENDER_FEMALE,
+        ];
+        if (! $gender in_array($genders)) {


This isn't valid code :)

Such things happen when you try to code in github...

As they are already used removing them is a BC break that we do not want!

This removes the calls to get/setSex with the appropriate get/setGender calls. Tests are now again passing

The removed docBlock contains redundant information and was therefore removed

heiglandreas · 2019-09-15T19:06:26Z

The removal of the protected proprety $sex as well as modifying the return-type of the method getSex from ?string to string is technically a BC break.

As the method now does no longer return null any checks expecting null can still be executed on the returned value, so it's not really going to break code.

And the removal of the $sex property only is a BC break for classes that extend the User-class. Currently that should not have happened. For the future I would advise to either mark the class as final or set the visibility of the properties to private instead of public or protected

See result of docker run --rm -v $PWD:/app nyholm/roave-bc-check

[BC] REMOVED: Property SocialConnect\Common\Entity\User#$sex was removed
[BC] CHANGED: The return type of SocialConnect\Common\Entity\User#getSex() changed from ?string to string
[BC] REMOVED: Property SocialConnect\Common\Entity\User#$sex was removed
[BC] CHANGED: The return type of SocialConnect\Common\Entity\User#getSex() changed from ?string to string
4 backwards-incompatible changes detected

ADmad reviewed Sep 15, 2019

View reviewed changes

heiglandreas and others added 7 commits September 15, 2019 18:55

Fix bugs

c6f183f

Such things happen when you try to code in github...

Reinstantiate the SEX_* constants

8b9b122

As they are already used removing them is a BC break that we do not want!

raise deprecation error when set/getSex is called

3c2950e

trigger error when deprecated constants are used

7985cb7

Remove calls to deprecated functions

7bb7cc3

This removes the calls to get/setSex with the appropriate get/setGender calls. Tests are now again passing

Fix CS-issues

cf0bb73

Remove unnecessary docblock

59da662

The removed docBlock contains redundant information and was therefore removed

ovr closed this Jan 29, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace get/setSex with get/setGender #95

Replace get/setSex with get/setGender #95

heiglandreas commented Sep 15, 2019

ADmad Sep 15, 2019

heiglandreas Sep 15, 2019

ADmad Sep 15, 2019

ADmad Sep 15, 2019 •

edited

Loading

heiglandreas commented Sep 15, 2019 •

edited

Loading

Replace get/setSex with get/setGender #95

Replace get/setSex with get/setGender #95

Conversation

heiglandreas commented Sep 15, 2019

ADmad Sep 15, 2019

Choose a reason for hiding this comment

heiglandreas Sep 15, 2019

Choose a reason for hiding this comment

ADmad Sep 15, 2019

Choose a reason for hiding this comment

ADmad Sep 15, 2019 • edited Loading

Choose a reason for hiding this comment

heiglandreas commented Sep 15, 2019 • edited Loading

ADmad Sep 15, 2019 •

edited

Loading

heiglandreas commented Sep 15, 2019 •

edited

Loading