java-design-patterns icon indicating copy to clipboard operation
java-design-patterns copied to clipboard

Refactor Layered Architecture

Open iluwatar opened this issue 1 year ago • 8 comments

Description

Layered Architecture example has some code smells.

CakeInfo - 'Optional<Long>' used as type for field 'id' CakeLayerInfo - 'Optional<Long>' used as type for field 'id' CakeToppingInfo - 'Optional<Long>' used as type for field 'id'

Acceptance Criteria

  • Code smells fixed

iluwatar avatar Jun 01 '24 12:06 iluwatar

Willing to work on this. However in my sts I am unable to see the code smells as such. Followed the instructions mentioned here :- https://github.com/iluwatar/java-design-patterns/wiki/12.-IDE-instructions

image Could only see identation issues as such. Please let me know how do I proceed to resolve this. P.S. - Interested to contribute to this repo !

ankurnotwarikoo avatar Jun 04 '24 05:06 ankurnotwarikoo

Don't know about other IDEs, but in IntelliJ IDEA you can reproduce the warnings by using command 'Analyze - Inspect Code' on the layered-architecture module.

iluwatar avatar Jun 04 '24 05:06 iluwatar

Could you assign this issue to me? This is my first time contributing, so I also need some guidance on how to commit and raise a pull request. Thank you!

ankurnotwarikoo avatar Jun 04 '24 09:06 ankurnotwarikoo

Sure @ankurnotwarikoo Here are a couple of good starting points to get started with Github and this project:

  • https://docs.github.com/en/get-started
  • https://github.com/iluwatar/java-design-patterns/wiki/01.-How-to-contribute

iluwatar avatar Jun 04 '24 10:06 iluwatar

@iluwatar - I am done with the code fixes. However, format the code using checkstlye plugin in Intellij and hence unable to surpass the validation of code. Do let me know if there's some checkstyle.xml on which defines the basis of formatting of file.

ankurnotwarikoo avatar Jun 05 '24 09:06 ankurnotwarikoo

Checkstyle is configured here: https://github.com/iluwatar/java-design-patterns/blob/master/pom.xml#L334-L356

It uses built-in Google Checks. We have added some of our own rule suppressions.

IDE instructions have some hints on how to work with Checkstyle https://github.com/iluwatar/java-design-patterns/wiki/12.-IDE-instructions

iluwatar avatar Jun 06 '24 05:06 iluwatar

Noticed that we have a partly duplicate issue https://github.com/iluwatar/java-design-patterns/issues/2936

iluwatar avatar Jun 06 '24 05:06 iluwatar

This issue has been automatically marked as stale because it has not had recent activity. The issue will be unassigned if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Aug 05 '24 06:08 stale[bot]

Can this issue be assigned to me?

abhishek-singh0710 avatar Oct 01 '24 03:10 abhishek-singh0710

Also I'm sorry if this seems like a poor question but are we only concerned with the optional type given to the ids of those three classes and none other?

abhishek-singh0710 avatar Oct 01 '24 03:10 abhishek-singh0710

I checked the concerned code files, I guess the code smells are fixed. The optional fields have been changed.

PALASH2201 avatar Oct 01 '24 03:10 PALASH2201

Ok. Thanks for letting me know.

abhishek-singh0710 avatar Oct 01 '24 04:10 abhishek-singh0710

Yea, no issues

PALASH2201 avatar Oct 01 '24 04:10 PALASH2201

Could you please assign this issue to me?

pujakarakoti07 avatar Oct 03 '24 13:10 pujakarakoti07

Fixed in https://github.com/iluwatar/java-design-patterns/pull/2988

iluwatar avatar Oct 13 '24 15:10 iluwatar