short icon indicating copy to clipboard operation
short copied to clipboard

[Feature] Make Filter structure in Search package more extendible

Open rohithbalaji123 opened this issue 4 years ago • 5 comments

What is the problem?

There is an inherent coupling between Resource and Order structures in Filter and it is quite difficult to extend when we try to add more resources or order by classes. That is., there is a requirement for every Resource to support each Order bys which will not be the case at all times.

When trying to add a new Resource:

  • We will need to add a method to the Order interface with the name ArrangeResource. That would mean that we need to implement those methods in all existing structures which implements Order(say CreatedTime ). But, the newly added Resource may not even have any field related to created time.

When trying to add a new Order:

  • When we have a requirement to add a new Order By(say a new column which tracks number of times the short link is visited), we cannot add this Order By as that would mean, we will have to implement all the methods in the Order interface (currently has ArrangeShortLinks, ArrangeUsers). But, this new Order By is related only to ShortLinks.

Your solution

Will have to think through to come up with a more generic solution.

Alternatives considered

For resources not supporting a particular order by, we can throw NotApplicable error when the package's client tries to order by the unsupported order by. This will also include validating in NewFilter that the resource and order arrays are compatible with each other.

rohithbalaji123 avatar Jun 24 '20 10:06 rohithbalaji123

@rohithbalaji123

  1. I don't think all resources has to support the same order. When can do unchanged order for those not supported. Validation is needed though.
  2. We don't have generics support in Go 1. Waiting for Go 2. It's already in experimentation stage

magicoder10 avatar Jun 24 '20 16:06 magicoder10

@byliuyang Yes, we can do unchanged order for unsupported resource. My concern is, what should ArrangeNewResource method's implementation be in CreatedTime structure when NewResource doesn't have a column called createdAt?

rohithbalaji123 avatar Jun 24 '20 18:06 rohithbalaji123

@rohithbalaji123 It doesn’t do anything? Is this avoidable?

magicoder10 avatar Jun 24 '20 18:06 magicoder10

@byliuyang , I am not sure how to avoid it. When we add more and more resources and Order By implementations, there will be a lot combinations where the method implementations will be empty.

rohithbalaji123 avatar Jun 24 '20 18:06 rohithbalaji123

We can also reverse things here, so each resource implements each order, and in case that an implementation is not possible we keep the slice unchanged. So we can have a resource type []entity.ShortLink which implements all the order types

arberiii avatar Jun 25 '20 09:06 arberiii