-
Notifications
You must be signed in to change notification settings - Fork 1
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bf 17 account page #65
Conversation
@RequestMapping("account_settings") | ||
public class UserProfileController { | ||
|
||
@Autowired |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Autowired here isn't necessary, as you can replace it by constructor injection which has alot of advantages as following:
-
the dependencies are clearly identified. There is no way to forget one when testing, or instantiating the object in any other circumstance (like creating the bean instance explicitly in a config class).
-
the dependencies can be final, which helps with robustness and thread-safety.
-
you don't need reflection to set the dependencies. InjectMocks is still usable, but not necessary. you can just create mocks by yourself and inject them by simply calling the constructor.
-
by the autowired approach, you are allowing anyone (in different class outside/inside the Spring container) to create an instance using default constructor (like new SomeService()), which is NOT good as you need SomeOtherService object (as a dependency) for your SomeService.
You may have some questions like "is this is applicable in the stereo type component as you don't call it?"
The answer is yes
and you can do it even for JPA repositories.
how does spring inject the dependency in this case ?
Spring scans your classes for constructor that matches your class' fields. Find details here:
Additional Details
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't prefer Autowired my self as well here but I decided to use it to be consistent with the rest of the project.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good, so we should mark it as an issue. I think it will need a huge refactoring, but it will be worthy.
var url = Uri.https(APIConstants.baseUrl, APIConstants.userProfileEndPoint); | ||
var response = await http.get( | ||
url, headers: APIConstants.headerCORS(session.cookie)); | ||
debugPrint(url.toString()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think, those debugging prints should be removed if you are finished.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you are right I forgot it.
Front-End
Back-End