Skip to content
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

Move: logger into new file #265

Closed
wants to merge 1 commit into from

Conversation

dexX7
Copy link

@dexX7 dexX7 commented Dec 30, 2014

This is actually a move only commit with the intention of isolating the logger, because it's a component basically used by every other, but currently bound to the huge mastercore main files.

Build confirmation:
https://travis-ci.org/dexX7/mastercore/builds/45404613

@dexX7
Copy link
Author

dexX7 commented Dec 30, 2014

I wanted to keep this as "move only", so consider this for further discussion.

What's most notable imho: it should not really be the decision of a specific module, whether something gets logged or not.

PSR-3 from the PHP world provides suggestions for an unified logger interface, which should not be used as blueprint, but might serve as idea:

public function log($level, $message, array $context = array());
...
public function warning($message, array $context = array());
public function info($message, array $context = array());
public function debug($message, array $context = array());

Here is an example for our case:

if (msc_debug_metadex1) {
    file_log("%s(%s: prop=%u, desprop=%u, desprice= %s);newo: %s\n", ...);
}

What I mean: x_Trade should neither worry, if msc_debug_metadex1 is enabled, nor if it's logged to a file, console or whatsoever. It might as well look like this:

Log("metadex1", "%s(%s: prop=%u, desprop=%u, desprice= %s);newo: %s\n", ...);

Using an enum or so instead of a string for the category or log level is to prefer, but this functionallity is basically already provided by Bitcoin Core with:

LogPrint(const char* category, const char* format, TINYFORMAT_VARARGS(n))

And it then gets decided, if given category is accepted or where the content to log is added to.

This should be a move only commit with the intention of isolating
the logger, because it's a component basically used by every other,
but currently bound to the huge mastercore main files.
@dexX7 dexX7 force-pushed the mcore-move-logger branch from d6e59d0 to 98cb064 Compare February 20, 2015 23:34
@whitj00
Copy link

whitj00 commented Mar 18, 2015

Ack, refactoring is good

@dexX7
Copy link
Author

dexX7 commented Apr 21, 2015

Addressed by OmniLayer#17.

@dexX7 dexX7 closed this Apr 21, 2015
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.

2 participants