Simplify and Modernize some aspects of Peruse
ClosedPublic

Authored by tcanabrava on Jun 3 2018, 5:37 PM.

Details

Summary

Simplify path logic

Code Style

Return a compile-time object

Instead of creating a temporary QStringList
appending some items to it and returning, I'm
creating a compile-time QStringList with compile
time QStrings.

Const Correctness on ACBF

too many files touched but this is just const-correctness

Reduce Temporaries

d-pointer with unique_ptr

const char * to QStringLiteral

Use compile-time lists instead of runtime generating it.

Also, simplify code - create a few helper functions to
return the result of the calculation instead of creating
them inside of the switch.

Create a Common static library to remove duplications

Diff Detail

Repository
R157 Peruse
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
tcanabrava requested review of this revision.Jun 3 2018, 5:37 PM
tcanabrava created this revision.
leinir requested changes to this revision.Jun 4 2018, 12:56 PM

A bunch of very good stuff in there :) Just a couple of comments, and one thing that i really would like to see changed, but all in all, very solid work :D

src/CMakeLists.txt
11

This was something i'd pondered on for a while and never got around to... Good call, and thank you :)

src/acbf/AcbfAuthor.h
26

std things scare me! (no, but really, i need to get used to those, they're cool :) ). We are absolutely certain this works on Windows, right?

src/contentlist/ContentQuery.cpp
184

Returns in the middle of a function make me very twitchy... but the refactor here seems to sort of mitigate my main concern. Still not entirely happy with it, though, I'd be much happier with an intermediary variable...

This revision now requires changes to proceed.Jun 4 2018, 12:56 PM
tcanabrava marked 2 inline comments as done.Jun 4 2018, 5:26 PM
tcanabrava added inline comments.
src/acbf/AcbfAuthor.h
26

it does with msvc > 2013 or with any mingw thing.
(std started to be good around c++11.)

src/contentlist/ContentQuery.cpp
184

no problem.

leinir added inline comments.Jun 5 2018, 7:14 AM
src/acbf/AcbfAuthor.h
26

Cool, really just wanted to be sure :) i vividly remember us doing some niftiness during the KO days and then discovering that the features weren't supported in msvc, which was... much fun ;) That, of course, was msvc2012, and things have moved considerably since then :)

tcanabrava updated this revision to Diff 35622.Jun 5 2018, 2:40 PM
tcanabrava marked an inline comment as done.
  • Don't return in the middle of a function
leinir accepted this revision.Jun 7 2018, 10:36 AM

Awesomesauce, go for it! :)

This revision is now accepted and ready to land.Jun 7 2018, 10:36 AM
This revision was automatically updated to reflect the committed changes.

arg, arc land squashed all the commits. u_u

Silly arc... Long as the message has all the comments and such, it's ok :) Thank you :D