-
Notifications
You must be signed in to change notification settings - Fork 1
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
Flyweight #28
base: master
Are you sure you want to change the base?
Flyweight #28
Conversation
import org.jetbrains.annotations.TestOnly | ||
|
||
class SoldierFactory { | ||
companion object { |
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 think this factory can be improved. It is so Java style...
val TYPE = 1 | ||
} | ||
init { | ||
Flyweight.objectInstances++ |
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.
Dangerous
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.
Seems okay overall 👍 Now you can look how to improve this code with Kotlin style 😉
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.
@@ -0,0 +1,19 @@ | |||
package oop.Flyweight | |||
|
|||
import java.awt.Point |
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.
Oh my fucking god ❌
|
||
class Admiral : Soldier { | ||
companion object { | ||
val TYPE = 1 |
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.
NOPE ❌
Flyweitght.objectInstances++ | ||
} | ||
|
||
val attack = 6 |
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.
NOPE ❌
} | ||
|
||
val attack = 6 | ||
val salary = 23000 |
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.
NOPE ❌
|
||
class Captain : Soldier { | ||
companion object { | ||
val TYPE = 2 |
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.
NOPE ❌
@@ -0,0 +1,7 @@ | |||
package oop.Flyweight | |||
|
|||
import java.awt.Point |
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.
NOPE ❌ NOPE ❌ NOPE ❌ NOPE ❌ NOPE ❌
@@ -0,0 +1,13 @@ | |||
package oop.Flyweight | |||
|
|||
import java.awt.Point |
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.
LOL data class Point(val x: Int, val y:Int)
|
||
class SoldierFactory { | ||
companion object { | ||
var admiral: Admiral? = null |
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.
kotlin? null? seriously? ❌
var admiral: Admiral? = null | ||
var captain: Captain? = null | ||
|
||
fun getSoldier(type: Int): Soldier{ |
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.
getSoldier(type: Int) lol Do you know what is a sealed class? 👍
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.
Do you know*
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.
Thanks @Cotel at least I can stop crying hard with this
return captain!! | ||
} | ||
} | ||
throw IllegalArgumentException() |
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.
xDDDDD fann1
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.
@Cotel Seems ok overall? |
Seems okay if you want to treat Kotlin like oldstyle Java, thats why I told him to update his code 🤣 |
I don't understand anything of this pattern. I've been reading but it is still unclear to me. Can you explain it? I need to know how to review this before I get to it 😵 |
The last one explains what is, in resume, when you have a In the @Nhemesy example, he wanted to have a Soldiers that if are the same "type" (same soldier) only create once because are two object with the same values. |
@Nhemesy What do you think? you need see the last changes. We need merge this |
@Nhemesy please |
Don't forget add Flyweight in README and complete with an example |
We are back! 💪 |
Edit by @tonilopezmr: Solved #19
Implemented flyweight pattern. Maybe you are thinking... why are you merging with state branch???
This is because the great @tonilopezmr said that Flyweight can be implemented with State, so I make pull to see if this can be done. And I cant see the similaritys between these patterns so, here's my version of flyweight! So much love <3