nice. Congrats to your first real C++ branch :) Obviously there are a few issues to fix. But you know how picky I am. Don't blame yourself for the length of the list. I intentionally explained lots of things.
====
Hmm... Why do you save the username? That just makes it more complex.
Instead of saving the username into the config, I would just save the token. This way, on startup we can just load the token from the config, set it to EvernoteConnection and not bother dealing with AccountService at all.
Once the token is set, the username will be automatically refreshed inside the UserStore.
====
Here's a couple of other things I'd like to have changed:
This works, but please align it to look the same as the CameraHelper. Apart from the visual difference, there is a theoretical issue with your code too: If you create a variable without "new", it will be allocated on the stack and automatically deleted when the function ends. If you do "new" you will allocate the object on the heap and need to delete it yourself. Now, given that the program is removed from the memory when the main() ends, this is really a theoretical issue here and doesn't really hurt us in practice. But I hope you get the idea and see the benefit of keeping things consistent with the existing code.
===
199 + var accountName = accountPreference.accountName;
200 + if (accountName) {
201 + var i;
202 + for (i = 0; i < accounts.count; i++) {
203 + if (accounts.get(i, "displayName") == accountName) {
204 + accountService.objectHandle = accounts.get(i, "accountServiceHandle");
205 + }
206 + }
207 + }
As said above, this should be more like:
if (accountPreferences.token) {
EvernoteConnection.token = accountPreferences.token;
} else {
// Do the AccountService stuff...
}
Heh, you're inside EvernoteConnection, you can directly access m_token instead of going through EvernoteConnection::instance()->token(). Also, never use NULL if a QString parameter is required. And one more: setToken() already does the emit tokenChanged(), you're emitting that twice now.
Make sure C++ code files always end with a newline. While this is again a theoretical issue in our case, it breaks compilation with some compilers, e.g. on Solaris.
Hey Riccardo,
nice. Congrats to your first real C++ branch :) Obviously there are a few issues to fix. But you know how picky I am. Don't blame yourself for the length of the list. I intentionally explained lots of things.
====
Hmm... Why do you save the username? That just makes it more complex.
Instead of saving the username into the config, I would just save the token. This way, on startup we can just load the token from the config, set it to EvernoteConnection and not bother dealing with AccountService at all.
Once the token is set, the username will be automatically refreshed inside the UserStore.
====
Here's a couple of other things I'd like to have changed:
134 CameraHelper helper; )->rootContext( )->setContextPr operty( "cameraHelper" , &helper); )->rootContext( )->setContextPr operty( "accountPrefere nce", new AccountPreferen ce());
135 view.engine(
136
137 + view.engine(
This works, but please align it to look the same as the CameraHelper. Apart from the visual difference, there is a theoretical issue with your code too: If you create a variable without "new", it will be allocated on the stack and automatically deleted when the function ends. If you do "new" you will allocate the object on the heap and need to delete it yourself. Now, given that the program is removed from the memory when the main() ends, this is really a theoretical issue here and doesn't really hurt us in practice. But I hope you get the idea and see the benefit of keeping things consistent with the existing code.
===
199 + var accountName = accountPreferen ce.accountName; objectHandle = accounts.get(i, "accountService Handle" );
200 + if (accountName) {
201 + var i;
202 + for (i = 0; i < accounts.count; i++) {
203 + if (accounts.get(i, "displayName") == accountName) {
204 + accountService.
205 + }
206 + }
207 + }
As said above, this should be more like:
if (accountPrefere nces.token) { nection. token = accountPreferen ces.token;
EvernoteCon
} else {
// Do the AccountService stuff...
}
====
372 +void EvernoteConnect ion::clearToken () ction:: instance( )->token( ).isEmpty( )) {
373 +{
374 + if (!EvernoteConne
375 + setToken(NULL);
376 + emit tokenChanged();
377 + }
378 +}
Heh, you're inside EvernoteConnection, you can directly access m_token instead of going through EvernoteConnect ion::instance( )->token( ). Also, never use NULL if a QString parameter is required. And one more: setToken() already does the emit tokenChanged(), you're emitting that twice now.
This function could (and should) be as short as:
void EvernoteConnect ion::clearToken () clear() ;
{
m_token.
emit tokenChanged();
}
or:
void EvernoteConnect ion::clearToken () QString( ));
{
setToken(
}
I guess the latter one is the better one.
====
56 \ No newline at end of file
Make sure C++ code files always end with a newline. While this is again a theoretical issue in our case, it breaks compilation with some compilers, e.g. on Solaris.
====
283 + text: i18n.tr("Accounts") app-symbolic" (true);
284 + iconName: "contacts-
285 + visible: accounts.count
286 + onTriggered: {
287 + openAccountPage
288 + }
289 + }
290 +
291 + ToolbarButton {
I personally don't think its good to have the button in the toolbar all the time, given that in practice you will never ever use it :)
How about putting it into the HUD? David, Dani, Alan, what's your opinion on this?