Dissem / Jabit

Bitmessage Library for Java
Apache License 2.0
26 stars 10 forks source link

Project architectural refactoring & java-docs #26

Open ghost opened 6 years ago

ghost commented 6 years ago

Now the project have lots of anti-patterns along with bad architectural design. Are there any refactorings planned?

Same question about java-docs, are there any plans to cover code with them?

Really respect you for the work done, but I suppose that it is important to complete these two steps.

Dissem commented 6 years ago

There are no specific plans, but the current move to Kotlin might be a good time for some refactoring. I would really appreciate your input: what do you consider the worst anti-patterns, and how would you resolve them?

As for the JavaDoc, I always strive to improve it, but I will not aim for 100% coverage - I really hate comments that just point out the obvious. Of course, apart from actual shortcomings, more things might be obvious to me, so the occasional rant might help improving things.

Thank you very much for looking at the code, I really appreciate that!

ghost commented 6 years ago

Currently, i am working on a migration to kotlin with some fixes.As for antipatterns, the worst here is signletone and god object. I'm trying to remove them with properly placed dependency injection and functional approach where possible. Another problem is delegating too much work to entities, it's better to use something like Visitor pattern.

Dissem commented 6 years ago

Please look at the existing Kotlin port (current 'develop' branch) first! It would be a shame to do the same work twice and I'm not sure I could easily accept a pull request if you do both at once.

But rest assured, I am very grateful for your efforts!

KomarovI notifications@github.com schrieb am Fr., 14. Juli 2017, 11:29:

Currently, i am working on a migration to kotlin with some fixes.As for antipatterns, the worst here is signletone and god object. I'm trying to remove them with properly placed dependency injection and functional approach where possible

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/Dissem/Jabit/issues/26#issuecomment-315315381, or mute the thread https://github.com/notifications/unsubscribe-auth/ABH4_MEPMso8KHV93c7mScDR0lnZJ9Vwks5sNzUHgaJpZM4OW-iT .

ghost commented 6 years ago

Let me provide an example of removing not needed complexity from entities. My approach is quite simple, let us delegate the work to some other components. As an example, let it be through two simple interfaces, the entity interface Streamable and its personal writer interface, StreamableWriter. Something like below would be enough to make the entity classes more clean:

interface Streamable {

    fun writer(): StreamableWriter
}
interface StreamableWriter {

    fun write(output: OutputStream)

    fun write(output: ByteBuffer)
}
class NetworkAddress(
        val time: Long,
        val streamNumber: Long,
        val services: Long,
        val ipv6: ByteArray,
        val port: Int
): Streamable {

    fun writer(light: Boolean): StreamableWriter = Writer(
            element = this,
            light = light
    )

    override fun writer(): StreamableWriter = Writer(
            element = this
    )

    internal class Writer(
            val element: NetworkAddress,
            val light: Boolean = false
    ): StreamableWriter {

        override fun write(output: OutputStream) {
            if(!light) {
                int64(element.time, output)
                int32(element.streamNumber, output)
            }
            int64(element.services, output)
            output.write(element.ipv6)
            int16(element.port.toLong(), output)
        }

        override fun write(output: ByteBuffer) {
            if(!light) {
                int64(element.time, output)
                int32(element.streamNumber, output)
            }
            int64(element.services, output)
            output.put(element.ipv6)
            int16(element.port.toLong(), output)
        }
    }
}

If you're interested in moving to approaches like this, I may rewrite some of your code to be more abstract and clear.

Dissem commented 6 years ago

Yes, this looks interesting. Please, go ahead! If you need any help, just ask, but it looks as if you understand my code quite well. It mightn't be that bad after all ;-)

ghost commented 6 years ago

It's not bad, but complex because of tasks complexity. Hope I'll complete core functional in two weeks. When done with it - I'll post a link here.

Dissem commented 6 years ago

I wanted to ask - did you get time to do some refactoring? Otherwise I might do it, I think the idea is great.

ghost commented 6 years ago

I've done some basic refactoring, but, unfortunately, was too busy to complete it. I'd send you a link to the project with sources after refactoring to help you a little bit. I grant you full access to read, write, use whatever you want from this archive, that contains full sources of what I've done. Not very much, but maybe you'd find something useful. Bitmessage-Backend-master-3e62a1accac855556f3eb548934d7216897f11b5.zip