ER: Synonym l for logger #188

Closed
JuergenSchuster opened this Issue Oct 20, 2016 · 15 comments

Projects

None yet

5 participants

@JuergenSchuster

Me again :-) you see I'm really working with the tool ;-)

I would like to have a default synonym l for the logger package.

So instead of 20 lines of: logger.log('xxxx'); -> l.log('xxx');

code looks so much cleaner then.

@zhudock
zhudock commented Oct 20, 2016

I'm not convinced that including a defaulted synonym in the Logger install is all that beneficial. However, if it was an optional "extra" script similar to the one included with oos-utils it might be OK.

Take a look here for an example of how to set this up yourself

@JuergenSchuster
JuergenSchuster commented Oct 20, 2016 edited

@zhudock why do you think saving a lot of sourcecode and make the logging lines more concise is not benificial?

It does not hurt to create a synonym, but if I do it all by myself I can not rely on it on other installations. So creating it by the installation you can use l.log instead of logger.log but you don't have to

@eaolson
eaolson commented Oct 21, 2016

I disagree. This decreases the readability of the code (l? What does l mean? Is that a 1?) for the sake of saving a few keystrokes.

@JuergenSchuster

@eaolson what das logger mean? 1ogger? Probably not ;-)

So "l" probably does not "mean" 1 but "l" and 1 means 1. In SQL-Developer and real editors you see the "el" is "el" and not I.

I'm not saying it should be this way for everybody, just give me the alternative which I can rely on on every logger installation.

This is my logger template without a single line of logic with and without logger:

procedure todo_proc_name( 
                          p_param1_todo in varchar2
                        , p_param2_todo in varchar2
                        ) 
is
  --
  l_scope logger_logs.scope%type := v('APP_ID') || '.' || v('APP_USER') || '.' || nvl(v('GID'),user) || '.' || g_scope_prefix 
                                    || 'todo_proc_name';
  l_params logger.tab_param;
  --
begin 
  --
  logger.append_param(l_params, 'p_param1_todo', p_param1_todo);
  logger.append_param(l_params, 'p_param2_todo', p_param2_todo);
  logger.log(' ');
  logger.log('START ' || l_scope, l_scope, null, l_params);
  logger.time_start('timer');
  --
  logger.log('Gathering all ARE for the analytic');
  null;
  logger.log('SQL: All ARE for the analytic S7 but only the live parameters');
  logger.time_start('SQL1');
  null;      
  logger.time_stop('SQL1');
  null;
  logger.log('Updating the customer records');
  null; 
  --
  logger.time_stop('timer');
  logger.log('END ' || l_scope , l_scope);
  logger.log(' ');
  --
exception
  when others then
    logger.log_error('Unhandled Exception: ', l_scope, null, l_params);
    logger.set_level('DEBUG',   coalesce(sys_context('userenv','client_identifier'),sys_context('userenv','session_user'))); 
    logger.log_apex_items(p_scope => l_scope)
    raise;
    --
    --
end todo_proc_name;

Without:

procedure todo_proc_name( 
                          p_param1_todo in varchar2
                        , p_param2_todo in varchar2
                        ) 
is
  --
  l_scope logger_logs.scope%type := v('APP_ID') || '.' || v('APP_USER') || '.' || nvl(v('GID'),user) || '.' || g_scope_prefix 
                                    || 'todo_proc_name';
  l_params l.tab_param;
  --
begin 
  --
  l.append_param(l_params, 'p_param1_todo', p_param1_todo);
  l.append_param(l_params, 'p_param2_todo', p_param2_todo);
  l.log(' ');
  l.log('START ' || l_scope, l_scope, null, l_params);
  l.time_start('timer');
  --
  l.log('Gathering all ARE for the analytic');
  null;
  l.log('SQL: All ARE for the analytic S7 but only the live parameters');
  l.time_start('SQL1');
  null;      
  l.time_stop('SQL1');
  null;
  l.log('Updating the customer records');
  null; 
  --
  l.time_stop('timer');
  l.log('END ' || l_scope , l_scope);
  l.log(' ');
  --
exception
  when others then
    l.log_error('Unhandled Exception: ', l_scope, null, l_params);
    l.set_level('DEBUG',   coalesce(sys_context('userenv','client_identifier'),sys_context('userenv','session_user'))); 
    l.log_apex_items(p_scope => l_scope)
    raise;
    --
    --
end todo_proc_name;
@zhudock
zhudock commented Oct 21, 2016

@JuergenSchuster

My thinking was similar to what @eaolson mentioned. It reduces the readability\clarity of the code.

It is certainly shorter code when using the synonym, but you lose the explicitly named package. That could potentially cause issues if\when a new developer takes over or joins a project.

I don't think the byte length of the source code has any effect on performance, so the preference would be to make the code as clear and explicit as possible simply for maintenance and future development reasons.

@martindsouza
Member

@JuergenSchuster I appreciate your feedback. I think that given the simplicity of this it's something that organizations can create and manage as they see fit.

As such we won't go ahead with this suggestion.

@JuergenSchuster

You all don't get the point, but this is something I have to live with ;-)

@martindsouza
Member
martindsouza commented Oct 21, 2016 edited

Just to be clear I do understand and actually think it's a really good idea to shorten logger. to l.. If I was the only one uses Logger I'd probably do this.

That said Logger is implemented in many systems. Each one of these have their own requirements etc. As a project we've somewhat reserved the logger name and can safely deploy to other systems. If we introduce a new synonym l that conflicts with others then it's just another issue to support and making it an optional feature install etc.

This may seem like a minor issue to most but again when installed in many systems I prefer to have as little impact as possible.

@jeffreykemp

Agreed with Martin on this, for the specific reason he's given - while I'd prefer to have the calls to logger be as "invisible" as possible to make the code more readable, Logger needs to be easily installable in any environment and cannot run the risk of clashing with other stuff.

I've made my own little customisations to Logger and there's nothing stopping you from creating a public synonym "l" for it - and it shouldn't be affected by later upgrades.

On a side note, I think I'd like to raise an enhancement request with SQL Developer to have an option to show logger calls differently (e.g. in a different colour, or a show/hide toggle).

@JuergenSchuster

@jeffreykemp Now we're talking :-) That was what I'm hoping for to take a mediocre idea from a non tech guy like me and take it to a whole new solution level by your brilliant brains. Do you talk to Kris or should I or both? @martindsouza

@martindsouza
Member

Glad that we can all agree on this now ;-) I've also made @jeffreykemp suggestion for the PL/SQL Atom plugin: OraOpenSource/language-oracle#24

@JuergenSchuster

@jeffreykemp can you write a small blog then I can post it on apex.world :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment