Skip to content

Conversation

@simplay
Copy link
Owner

@simplay simplay commented Mar 15, 2019

This patch moves all log-methods to a common place. The idea of this patch is to reduce the pollution of classes.

@simplay simplay changed the title (WIP) Rework logging Rework logging Mar 15, 2019
@simplay
Copy link
Owner Author

simplay commented Mar 15, 2019

@panmari : What are your thoughts on this change? Is this rather confusing or does it indeed reduce the overall pollution? Personally, I make use of these log-method quiet often (when debugging code via gdb).

class MeshTriangle;
class MeshData;

class Logger {
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the final idea is to make this a singleton class. Currently, this is stateless. Later, this class could however store the path to its log file (or even an output stream).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it! Having logging code in one place makes sense to me.

nb.log();
nc.log();
}
Vector3f getNormalA() const { return na; }
Copy link
Owner Author

@simplay simplay Mar 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we could omit these getter-method and simply expose na, nb and nc since there are immutable (marked by const), then again this could produce an inconsistent api. One might assume that are getters. Dunno what makes sense the most. Any suggestions @panmari ?

Copy link
Contributor

@panmari panmari Mar 17, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whatever your preferred style is. Be aware that this is more than a getter though: It makes a copy of na.

@simplay simplay force-pushed the rework-logging branch 3 times, most recently from c357fdd to 34596c1 Compare March 15, 2019 18:59

TEST(Logger, log_ray) {
Ray ray(new Vector3f(), new Vector3f());
Logger().log(ray);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make this better testable, you could split the method into string conversion/actual logging, i.e:

ToString(MeshTriangle&) {
}

template
Log(const Loggable& l) {
std::out << ToString(l);
}

@simplay simplay merged commit 2ae477e into master Mar 18, 2019
@simplay simplay deleted the rework-logging branch March 18, 2019 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants