BellevueCollege / CtcApi

A standardized, reusable, open source code library developed by and for the Community & Technical Colleges of Washington State in the creation of their own software applications.
Other
9 stars 3 forks source link

Add LogElapsed class to the API #10

Open cmshawns opened 11 years ago

cmshawns commented 11 years ago

Rewrite the LogElapsed class I've (quickly) written for a VB project in C# and add it to the toolkit in the API core.

See gist #7018279 for the VB code.

dmielcarek commented 11 years ago

I will take this on and update when complete.

dmielcarek commented 11 years ago

Suggestion C# Code:

using Common.Logging;

public class LogElapsed : IDisposable
{
    DateTime _start = DateTime.Now;
    ILog _log = LogManager.GetCurrentClassLogger();
    String _msg = "";
    public void Dispose()
    {
        TimeSpan elapsed = DateTime.Now - _start;
        _log.Debug(String.Format("Elapsed: [{1}] - for {0}", _msg, elapsed));
    }
    void New(String ctcAPISParam)
    {
        _start = DateTime.Now;
        _log.Trace(String.Format("Starting TIMER for: {0} ({1})", ctcAPISParam, _start.ToString("hh:mm:ss.ffff tt")));
        _msg = ctcAPISParam;
    }
}
cmshawns commented 11 years ago

Looks like you've still got some VB syntax in there - specifically, around the constructor. I also changed name of the parameter being passed to the constructor to be more descriptive:

using Common.Logging;

public class LogElapsed : IDisposable
{
    DateTime _start = DateTime.Now;
    ILog _log = LogManager.GetCurrentClassLogger();
    String _msg = "";

    public void Dispose()
    {
        TimeSpan elapsed = DateTime.Now - _start;
        _log.Debug(String.Format("Elapsed: [{1}] - for {0}", _msg, elapsed));
    }

    public LogElapsed(String label)
    {
        _start = DateTime.Now;
        _log.Trace(String.Format("Starting TIMER for: {0} ({1})", label, start.ToString("hh:mm:ss.ffff tt")));
        _msg = label;
    }
}

I'd also suggest:

I could go on and on :-) but this is a good start.

dmielcarek commented 11 years ago

Guess I might have been confused. I thought you just wanted a straight one-to-one conversion from VB to C#. It sounds like from your comment that you were also looking for upgrades/changes? If so, that is not the approach I took.

Also, I compiled the code in Visual Studio 2012, using C# and .Net 4. It seemed to compile fine. What syntax is VB? The compiler didn't throw any issues..

cmshawns commented 11 years ago
 Public Sub New(msg As String)

is VB syntax. Honestly, I'm surprised it compiles and runs under C#, but I'd guess what you've actually done is overridden the new operator - as opposed to providing a constructor for the class.

It's the functionality that I want to add to the API. The VB code I provided is just a guide - a quick thing I worked up for the VB app I'm currently working on.

dmielcarek commented 11 years ago

Now I am really confused. I just checked my local code and my comment and see no reference to: Public Sub New(msg As String)

My code has: void New(String ctcAPISParam)

cmshawns commented 11 years ago

Yes.

Public Sub New(msg as String)

is VB syntax for defining a constructor. The proper syntax for defining a constructor in C# is to use the name of the class:

public LogElapsed(String msg)

The class you posted would compile, but did you try to use it? The following command - in C# - would attempt to execute a constructor named LogElapsed(), not one named New().

LogElapsed foo = new LogElapsed("my message");
dmielcarek commented 11 years ago

I think there is a break in communication somewhere. There is 'no' VB code in my comments/code. Also my comment/code doesn't contain a 'LogElapsed' function. It is like you are referring to your own code/comments, or?

At this point, I think a verbal exchange would be better, as something is missing in this process. Just let me know if you want to discuss on the phone or I can just back out of this issue?

cmshawn commented 10 years ago

I'll take care of adding this.