search

SOA-04 / leaf-api

0 stars 3 forks source link

DEVELOP 05: Domain Modeling #67

Open jerry914 opened 1 week ago

jerry914 commented 1 week ago

Task

I couldn't find your Heroku Web App URL in the Google Team Sheet. Please ensure your application is deployed online. Feel free to ask for assistance on Teams if you need any help.

RuboCop/Code Smell Offenses

N/A

You're almost there—keep up the great work!

soumyaray commented 3 days ago

https://github.com/SOA-04/leaf-api/blob/eb306464100f5a9e657c6cf974aad72974c5364a/app/domain/plan/entities/plan.rb#L30

Your domain entities should not be calling outside infrastructure! Please make sure their mappers give them all the data they need to process for business logic.

soumyaray commented 3 days ago

https://github.com/SOA-04/leaf-api/blob/eb306464100f5a9e657c6cf974aad72974c5364a/app/domain/plan/entities/plan.rb#L49-L50

I feel that new_distance and new_duration should simply be methods called distance and duration that are computed on demand. See earlier comment that all the data they need should be retrieved from APIs by mappers before this entity is called.

soumyaray commented 3 days ago

https://github.com/SOA-04/leaf-api/blob/eb306464100f5a9e657c6cf974aad72974c5364a/app/domain/plan/utils.rb#L6

How about giving this module an interesting name like DistanceCalculator or something similar? Another thought is that some (or all?) methods could simply go in an entity/value like location because it uses its attributes? That would constitute business logic!

soumyaray commented 3 days ago

https://github.com/SOA-04/leaf-api/blob/eb306464100f5a9e657c6cf974aad72974c5364a/app/domain/query/entities/query.rb#L9

Not sure why this entity is generically called Query – should it be perhaps SearchedPlans or something like that?

pedestrianlove commented 1 day ago

I couldn't find your Heroku Web App URL in the Google Team Sheet. Please ensure your application is deployed online. Feel free to ask for assistance on Teams if you need any help.

 It's here: https://soa04-leaf-d708d21ee0ae.herokuapp.com/ image

pedestrianlove commented 1 day ago

Your domain entities should not be calling outside infrastructure! Please make sure their mappers give them all the data they need to process for business logic.

Maybe we'll try to refactor this after we finished the refactor-application homework.

I feel that new_distance and new_duration should simply be methods called distance and duration that are computed on demand. See earlier comment that all the data they need should be retrieved from APIs by mappers before this entity is called.

I originally thought that storing extra information on the entity will be beneficial for the performance, so instead of computing on demand, the distance/duration attribute will be calculated in advance.

How about giving this module an interesting name like DistanceCalculator or something similar? Another thought is that some (or all?) methods could simply go in an entity/value like location because it uses its attributes? That would constitute business logic!

That part is currently a dead code, so we're still figuring out how to make good use of it. Maybe we'll figure out what's the better name for the module.

Not sure why this entity is generically called Query – should it be perhaps SearchedPlans or something like that?

I originally want something brief and easy to type/put in address and not making the url too long, so I chose query as the name for the entity.