-
Create an insert method overload that allows for inserts without logging, and have your logging class use that. Otherwise, your design is recursive by definition: Log all DB inserts by performing a DB insert.
-
add an optional parameter to doInsert $callerIsLogger=false, then everytime you need to doInsert() from your logger, provide the second parameter true, and your doInsert could check for this situation and not call the logger when is called by logger. What hierarchies? KISS methodology rocks :)
-
This is a good candidate for the Mediator Pattern. You would have an object that the logger and database would both invoke various methods on, and this mediator object would keep references to the logger and database and handle communication for you.
-
Create a Logger class that implements a interface ILogger. A new instance of the Logger class receives a Object that implements the interface ILoggingProvider that is used to output log messages.
Create a Database class that implements the ILoggingProvider interface. A new instance of the Database receives a object that implements the ILogger interface that is used to log messages.
public interface ILogger
{
void Debug(String message)
}
public interface ILoggingProvider
{
void Log(String message)
}
public class Logger : ILogger
{
private ILoggingProvider LoggingProvider { get; set; }
public Logger(ILoggingProvider loggingProvider)
{
this.LoggingProvider = loggingProvider;
}
public void Debug(String message)
{
this.LoggingProvider.Log(message);
}
}
public class Database : ILoggingProvider
{
private ILogger Logger { get; set; }
public Database(ILogger logger)
{
this.Logger = logger;
}
public void DoStuffWithTheDatabase()
{
// Do stuff with the database
this.Logger.Debug("Did stuff with the database.");
}
public void Log(String message)
{
// Store message to database - be carefull not to generate new
// log messages here; you can only use the subset of the Database
// methods that do not themselve generate log messages
}
}
-
It sounds like you have two different database accesses - logged (normal case) and unlogged (for the logging functions). Split the database class into two, a higher-level version for logged requests, and a lower-level version for unlogged requests. Implement the higher level database class using references to the logging and lower level database classes.
-
You could add another class that both the Database and Logger use to avoid getting into an infinite logging loop (and avoid the circular dependency as well).
// Used only by Logger and DatabaseProxy
class Database {
function doInsert( $sql ) {
// Do the actual insert.
}
}
class Logger {
function log($sql) {
$msg_sql = ...
Database.doInsert($msg_sql);
}
// Used by all classes
class DatabaseProxy {
function doInsert( $sql ) {
Logger.log($sql);
Database.doInsert($sql);
}
}
You could dress it up a bit by having both Database and DatabaseProxy implement a common interface as well, and use a factory to provide the appropriate instance.
-
You have combined too many things together.
The database can't really depend on a logger which depends on the database. It isn't good design.
What you really have are two kinds of database access.
Low-level access does "raw" SQL.
The logger can depend on this lower-level class. It doesn't -- itself -- have raw SQL in it. It depends on a lower-level class.
High-level access does application queries, it uses low-level access and the logger.
-
If you want to give the database object access to a logger, then you pass the logger to it. If you want to give your logger access to a database object, then you pass the database object to it.
Check to see if those objects exist within the class before you use functions that are provided by them.
Since PHP passes these objects by reference by default, you can do this with a single logger and database class for several objects.
I'd also recommend you do all of your database writing at a single point in the logger. Just store everything in a temporary array and run your DB code when you deconstruct. Otherwise, you'll create a ton of overhead if you're writing to the database throughout your code.
class Database {
private $_logger = 0; //contains your logger
/* The rest of your class goes here */
//Add the logger to your class so it may access it.
public function setLogger($logger) {
$this->_logger = $logger;
}
//To check if we have a logger, we don't want to use one if we lack one.
public function hasLogger() {
if($this->_logger) return true;
else return false;
}
}
class Logger {
private $_database = 0; //variable to hold your database object
/* The rest of your class goes here */
public function setDatabase($database) { //Same crap basically
$this->_database = $database;
}
public function hasDatabase() {
if($this->_database) return true;
else return false;
}
public function doSomething { //This is new though
if($this->hasDatabase()) { //Shows how you use hasDatabase()
$this->_database->someDatabaseFunction();
}
else {
//Whatever you'd do without a database connection
}
}
}
$database = new Database;
$logger = new Logger;
$database->setLogger($logger);
$logger->setDatabase($database);
-
IMHO, the database class should not be interacting with the logger. I would be inclined to call the logger from the same part of the code that is calling the database class. Then the logger can use the database class to do its inserts.
-
Decouple Database from Logger. I would design the application to log from the middle tier and make the decision about which logger type to use at
runtime not compile time. I.e. use a factory method to determine whether to log to db/xml/whatever.
If the Data Layer does need to log (i.e. report a problem) have it throw an exception, catch it in the middle tier and then decide how to handle it there or hand it off to a diagnostics class and have that decide. Either way I'd keep the DAL as "blind/dumb" as possible and not have it making decisions about what is and what is not a loggable event.
A common parent class is not a good idea. A logger is not a database[r]. And nor is a database a logger. Its not so much a chicken and egg problem as it is a cow and a pig problem. They are two different animals. There is no reason for Database to be aware of logger. I think you are forcing the abstraction. All i see is that a Logger has a Database.... if it's a db logger that is.
You drive the data layer from the middle tier anyway, so when including exceptions and events, I cant see where you would lose fidelity in logging db related events.
public interface ILoggingProvider
{
void Log(String message)
}
public static class Logger
{
public static ILoggingProvider GetLoggingProvider()
{
//factory to return your logger type
}
public void Log(String message)
{
Logger.GetLoggingProvider().Log(message);
}
}
public class DBLogger : ILoggingProvider {
void Log(string message) {
Database db = new Database();
db.doInsert(someLoggingProc);
}
}
public class Database
{
runQuery(sUpdateQuery)
doInsert(sInsert)
}
...
public class Customer{
public void SaveCustomer(Customer c)
{
try {
// Build your query
Database db = new Database();
db.runQuery(theQuery);
} catch (SqlException ex) {
Logger.Log(ex.message);
}
}
}
-
sounds like you adding a lot of complication to a simple problem purely for the sake of making it object oriented...
ask yourself this, what are you really gaining by taking that approach?
-
You could make a second arg for your Database::insert() function:
function insert($sql, $logThis = true) { ... }
And then obviously when the logger calls it, make the second argument false.
Alternatively, just check the function stack to see if the insert function was called from the Logging class.
function insert($sql) {
$callStack = debug_backtrace();
if (!isset($callStack[1]) || $callStack[1]['class'] !== "Logger") {
Logger::logSQL($sql);
}
// ...
}
-
How I would do it:
public interface ILogger
{
void LogWarning(string message);
void LogMessage(string message);
}
public interface ILoggingProvider
{
void LogEntry(string type, string message);
}
public interface IDbProvider
{
void DoInsert(string insertQuery, params object[] parameters);
}
public class Logger: ILogger
{
private ILoggingProvider _provider = ProviderFactory.GetLogProvider();
public void LogWarning(string message)
{
_provider.LogEntry("warn", message);
}
public void LogMessage(string message)
{
_provider.LogEntry("message", message);
}
public void LogEntry(string type, string message)
{
_provider.LogEntry(type, message);
}
}
public class Database : IDbProvider
{
private Logger _logger = new Logger();
public void DoInsert(string insertQuery, params object [] parameters)
{
_logger.LogEntry("query", insertQuery);
}
}
public class DbLogProvider : ILoggingProvider
{
private IDbProvider _dbProvider = ProviderFactory.GetDbProvider();
public void LogEntry(string type, string message)
{
_dbProvider.DoInsert("insert into log(type,message) select @type,@message",type,message);
}
}
public static class ProviderFactory
{
public static IDbProvider GetDbProvider()
{
return new Database();
}
internal static ILoggingProvider GetLogProvider()
{
return new DbLogProvider();
}
}
Although I would probably also make it look up a type from a config file rather than hardcoding them in the ProviderFactory class. The assumption here is that your code doesn't care how it is logged, that's up the administrator, and that all logging would be done one way during execution(which would my preference anyway). Obviously you could extend this to make a choice as to the log target when creating the logger and create the appropriate provider.
