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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor server.js:server/server.js #26

Open
Purukitto opened this issue Aug 28, 2019 · 10 comments

Comments

@Purukitto
Copy link
Owner

commented Aug 28, 2019

I've selected server.js:server/server.js for refactoring, which is a unit of 16 lines of code. Addressing this will make our codebase more maintainable and improve Better Code Hub's Write Short Units of Code guideline rating! 馃憤

Here's the gist of this guideline:

  • Definition 馃摉
    Limit the length of code units to 15 lines of code.
  • Why
    Small units are easier to analyse, test and reuse.
  • How 馃敡
    When writing new units, don't let them grow above 15 lines of code. When a unit grows beyond this, split it in smaller units of no longer than 15 lines.

You can find more info about this guideline in Building Maintainable Software. 馃摉


鈩癸笍 To know how many other refactoring candidates need addressing to get a guideline compliant, select some by clicking on the 馃敳 next to them. The risk profile below the candidates signals () when it's enough! 馃弫


Good luck and happy coding! :shipit: 馃挴

@issue-label-bot

This comment was marked as resolved.

Copy link

commented Aug 28, 2019

Issue-Label Bot is automatically applying the label feature_request to this issue, with a confidence of 0.95. Please mark this comment with 馃憤 or 馃憥 to give our bot feedback!

Links: app homepage, dashboard and code for this bot.

@BrooksPatola

This comment has been minimized.

Copy link

commented Aug 28, 2019

can i take?

@Purukitto

This comment has been minimized.

Copy link
Owner Author

commented Aug 29, 2019

can i take?

Sure @BrooksPatola go ahead :) Let me know if you need any help

@BrooksPatola

This comment has been minimized.

Copy link

commented Aug 29, 2019

@Purukitto I had a bit of trouble doing the refractor. Let me know what you think of the current implementation.

@TantumErgo

This comment has been minimized.

Copy link

commented Sep 4, 2019

@Purukitto Two questions:

  1. You said there's 16 lines of code. Does that mean the non-whitespace lines from line 13-39?

  2. Are you ok with creating more than one file to accomplish what is currently being done in this one file? If you were to split the file roughly in half, and put the code from lines 27-39 in a separate file and then call it from this current file, that could potentially work. However, doing this will increase the number of files in the project.

@Purukitto

This comment has been minimized.

Copy link
Owner Author

commented Sep 5, 2019

  1. You said there's 16 lines of code. Does that mean the non-whitespace lines from line 13-39?

Yes that's right

  1. Are you ok with creating more than one file to accomplish what is currently being done in this one file?

That would be good way to shorten the length of the file. Just make sure you follow proper file and variable naming convention.

@BrooksPatola

This comment has been minimized.

Copy link

commented Sep 5, 2019

I submitted a PR using || short circuit operator which works on my end. Let me know if it does on yours.
#34

@Purukitto

This comment has been minimized.

Copy link
Owner Author

commented Sep 5, 2019

I submitted a PR using || short circuit operator which works on my end. Let me know if it does on yours.
#34
CC: @pranjaldatta

@TantumErgo

This comment has been minimized.

Copy link

commented Sep 5, 2019

@BrooksPatola #34 should work, well done.

@BrooksPatola

This comment has been minimized.

Copy link

commented Sep 5, 2019

@TantumErgo Thanks.

@Purukitto will you be able to merge my PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can鈥檛 perform that action at this time.