CS5500-S-2023 / team-bear

team-bear created by GitHub Classroom
0 stars 0 forks source link

wrapped up adding javaDoc and refactoring NewGuildJoined #55

Closed tsanevp closed 1 year ago

tsanevp commented 1 year ago

Changes made:

CommandModule.java:

CreateListingCommand.java:

NewGuildJoined.java:

User.java/UserController.java:

lenad90 commented 1 year ago

I think if the variable is used more than once, than it's a must. But some variables are hard to understand with just the variable name, and I think it would be easier for readability to have it back in the code just so that it doesn't have to require the reader to refer back to the top class to see what the variable is.

tsanevp commented 1 year ago

This is true, but some of the strings are super long. I don't mind adding them back in, but if you hover over the variable name, it shows the whole string, so you don't need to refer back to the top. If you still think we should add them back in, I am all for it and will update that before I merge.

lenad90 commented 1 year ago

@tsanevp Oh I didn't know about the hover! That does make it easier. I'm okay with either, but this stackoverflow thread might help with your decision: https://stackoverflow.com/questions/4234129/should-i-use-constants-instead-of-strings-even-if-the-strings-are-only-ever-used

tsanevp commented 1 year ago

Okay, based on that and your input, I'll only factor out the strings that occur more than once. Otherwise I agree that it might make it hard to understand the string being sent.