<table><tr><td style="">bruns marked 2 inline comments as done.<br />bruns added inline comments.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D21041">View Revision</a></tr></table><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21041#inline-118227">View Inline</a><span style="color: #4b4d51; font-weight: bold;">hallas</span> wrote in <span style="color: #4b4d51; font-weight: bold;">fstabdevice.cpp:62</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Would it make sense to wrap this in a function with a more expressive name like: isEncryptedFilesystem or isValutFilesystem? Then we could use the same function in fstabhandling.cpp:126</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I don't think there is much to gain here - as the function is trivial, we should inline it. To keep the code easy to understand, the function name has to be really expressive. On the next fuse filesystem added, it may become quite different.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21041#inline-118225">View Inline</a><span style="color: #4b4d51; font-weight: bold;">hallas</span> wrote in <span style="color: #4b4d51; font-weight: bold;">fstabdevice.cpp:172</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Why do we create a new FstabNetworkShare everytime this function is called with NetworkShare but we return the same object instance when it is a StorageAccess?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">The storageAccess is also used internally, for the <tt style="background: #ebebeb; font-size: 13px;">emblems()</tt> method. Creating it on each call would be to expensive, especially as it has quite some signals connected, also DBus signals.<br />
NetworkShare is quite lightweight. There is no reason it couldn't be cached, but hardly any benefit.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R245 Solid</div></div></div><br /><div><strong>BRANCH</strong><div><div>fstab_generic</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D21041">https://phabricator.kde.org/D21041</a></div></div><br /><div><strong>To: </strong>bruns, Frameworks, ngraham, hallas<br /><strong>Cc: </strong>broulik, kde-frameworks-devel, michaelh, ngraham, bruns<br /></div>