Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Immutable Vector 2D #161
Immutable Vector 2D #161
Changes from 16 commits
08b121c
05b49d9
0746b62
2d4eb3c
98823c4
7403c9d
429e094
1f66974
58c4cbb
7d9f0c4
9ef346d
446afb2
008693f
c388137
f1d51eb
ca6364b
71875f7
d4b51a7
9de52b1
ac08485
9a19a62
c4d8a2e
9279fee
1c0070e
063a017
455ce76
4d29ad3
dbdf93b
b95c6ad
cdc72c9
06909b8
90f7186
c1bb643
99b02c8
18d87eb
3cb53ad
a9456b7
43c9e58
6997b62
8a58587
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you rename the
l2
parameter tolengthSquared
for readability? I want the sources to be easy to read for Kotlin/LibGDX beginners.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed for the folowing:
Is that ok?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd generally tend to rewrite the conditionals to end up with
else this
, but it's subjective. Since there are two checks and slightly nested series of calls, I'd consider breaking the one-liner and using braces. Something along the lines of:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand that these were the method parameters in the original LibGDX code, but I'm not a fan of such non-obvious names. You could use
x
,factorX
orxFactor
instead.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Me neither. Actually in LibDGX they were named
x
andy
. But i tend to find it even less obvious.I'll go for
factorX
. (same for the methods bellow)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, non-obvious names.
x
,otherX
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same goes for a few methods below.