Skip to content
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

[WIP]refactor: refactor java-sdk modules #7

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

kevinten10
Copy link

@kevinten10 kevinten10 commented Dec 12, 2021

Hi, now I am making this part codes,

@seeflood can you review this?(when you have time)

I have added some module introductions below,
Looking forward to your suggestions, I will take time to discuss and refactor my code.

Thank you so much!

@kevinten10
Copy link
Author

Here is the overall design of the module:

           sdk-runtime/sdk-reactor
           ↑                       ↑ 
    sdk-domain         ->       sdk-grpc
          ↑                        ↑
sdk-infrastructure              spec-pb

@kevinten10
Copy link
Author

sdk-infrastructure:

  • basic properties
  • basic exceptions
  • basic serializer
  • basic utils
  • basic values/enums

@kevinten10
Copy link
Author

sdk-domain:

  • API entities
  • Blocking Runtime API interface
  • Non-Blocking Runtime API interface (extends Blocking Runtime API interface, And add default blocking call by block())

And I try using reactor-core as compile scope, so that Blocking user can run without reactor-core if they not use rx API.
see #3

@kevinten10
Copy link
Author

spec-pb:

API generated by protobuf

@kevinten10
Copy link
Author

sdk-grpc:

  • grpc stub
  • grpc callback
  • grpc utils

@kevinten10
Copy link
Author

sdk-runtime:

  • implements RuntimeClient
  • defined customer serializer

sdk-reactor:

  • implements ReactorRuntimeClient
  • defined customer serializer

@seeflood
Copy link
Member

seeflood commented Dec 17, 2021

Thanks for your contribution!
I'm reading your code.
This is a big PR that may affect existing users in our production environment. @ujjboy @zhenjunMa @ccx1024cc Could u help review this PR together?

@seeflood
Copy link
Member

seeflood commented Dec 17, 2021

So the dependency graph is :

           sdk-runtime/sdk-reactor
                                    ↑ 
    sdk-domain         ->       sdk-grpc
          ↑                        ↑
sdk-infrastructure              spec-pb

Right?

@seeflood
Copy link
Member

seeflood commented Dec 18, 2021

不好意思现在才回,我用中文说吧,感觉这个PR的设计有点复杂了,没法满足sdk的设计目标

sdk的设计目标

1.普通的sdk模块要足够简单,简单到非java选手学一下java语法就能上手开发

目前sdk里的代码很多就是非java开发者写的,比如 @MoonShining @ccx1024cc 他们在学习java语法后都能写出很好的sdk代码。

举个例子,现在sdk代码里有很多用数组的地方,其实这不是java风格,java常用List。但是这无所谓,这些数组是非java同学贡献的,并不影响使用。

那么怎么判断代码是否"简单"?衡量标准就是:非java选手想要给普通的sdk模块加功能时,能快速看懂,快速上手开发。

因此要减少开发者打开搜索引擎搜东西的次数。比如不往里面放maven相关的trick(最好不要让一个非java选手打开搜索引擎去搜maven这些配置项干嘛的,他只需要知道java语法),比如能不抽象尽量不抽象,不做一些DDD之类的领域建模、分层(我不是说建模、分层不好,只是这些技巧适合用在复杂系统里,不适合用在这个sdk)再比如一些高级语法糖也是尽量不用。

这也是为啥想把reactor相关的代码和普通sdk的代码放在不同模块:非java选手在上手开发(普通的sdk模块)时,只看非reactor模块的代码,不需要打开搜索引擎去搜reactor相关的知识,也不需要去搜索maven的配置项是啥意思

题外话:java骂娘定律

就经验来说,让非java开发者来写java代码,大家每需要靠搜索引擎多学习一个新知识点,心中的怒火就会越旺。最后写完一个功能后,大家的评价都是:java, 垃圾
这不是我的观点,我是java开发者,哈哈 :)

2.隔离: 让 "简单的部分" 和 "极客的部分" 隔离开

当然,这不代表我们不想往sdk里添加复杂度,我最近也在学reactor,挺有意思,感觉和nodejs的编程风格是相通的。

所以我个人觉得,我们当然可以往里面添加复杂度,但需要把"简单的部分"和 "极客的部分" 隔离开,让非java的beginner不需要点开"极客的部分"就能看懂代码、往里面添加自己想要的功能。

这样我们就能随意往 "极客的部分" 添加复杂度(比如用各种java高版本的语法糖,甚至干脆只支持java 11),省的去考虑一些兼容问题,或者非java选手的感受。

这样隔离之后,也能让社区对"极客的部分"足够自治。比如邀请你作为这部分的reviewer,你来对PR把关,有新PR你说了算 :)

具体模块分层

基于上述原则,我个人的想法是:

image

解释:

simple enough for non-java developers

  1. 对于左侧红框标注部分,简单到让非java开发者也能快速上手,不让他打开搜索引擎搜各种知识,能不抽象尽量不抽象
  2. sdk-infrastructure这个包名看着有点难理解、看不出来和domain的边界,能否和domain放一起,减少理解成本

geeks for geeks

  1. 对于右侧蓝框部分,右侧是社区自治。各位感兴趣的资深java选手,谁想往里面加一些geek的东西都行,保证和左侧隔离

这只是我个人想法,如果您觉得不妥、有其他诉求(比如想拆出来一个sdk-grpc包、用来给别的项目import?)咱们都可以商量哈,只要能满足上面说的设计目标即可。

如果您觉得打字沟通效率太低,咱们可以搞个社区视频会议聊一下

<parent>
<artifactId>runtime-sdk-parent</artifactId>
<groupId>io.mosn.layotto</groupId>
<version>1.1.0-SNAPSHOT</version>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could u add a version property to manage all the pom version?

@kevinten10
Copy link
Author

@seeflood 非常感谢你的建议!是的,你说的没错,我已经意识到了这一点。

目前我正在将rx相关内容从核心逻辑中剥离出来,尽量保持原sdk不变,依赖关系图基本如你所示。

我会在完成这部分更改后,将代码提交到这个PR中。

非常感谢你的review,让我们保持讨论。

image

@kevinten10
Copy link
Author

Hi,我调整后的代码设计和你的图示有两点不同:

  1. 独立的sdk-grpc包,按您的意思是"保持简单",是需要将grpc移动回sdk包中?这样我需要在sdk-reactor包中再copy一份grpc的代码,因为我看来grpc是通讯层,和sdk层的同步异步没有太大关系,所以作为独立模块供两个sdk使用也是ok的,这一点我们需要讨论一下。
  2. 一些通用功能(异常定义,序列化,工具类等),在底层的infrastructure层,这一点在"保持简单"上应该是可容许的建模?所以这一层我倾向于保留,您认为呢?

@kevinten10
Copy link
Author

这是一个Big PR,我仅在这里展示我做出的所有改动。

当讨论过后确定改动时,我会分成两个PR再次进行提交:

  1. 对原有sdk的改动
  2. 在原有sdk上的reactor拓展(对原sdk无影响)

您认为这样OK吗?

@seeflood seeflood self-assigned this Dec 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants