|
Yes, but I don't think that the disk access i the bottle neck. Besides, if you read a file it will be in the disk cache, so next time you will actually be reading it from memory. And if you store too much in memory, some of it will be swapped to disk, so you will be reading some images from disk anyway...
Have you checked how much network bandwidth you are using?
Despite everything, the person most likely to be fooling you next is yourself.
|
|
|
|
|
Thanks Guffa!
The bandwidth between machine is 1G since it is network center internal high speed machine connection between the same sub-network.
Previous the images are stored in database as blob data type, it is several hundred M bytes (about 1G) footprint, I think if I could cache them all into memory, the performance could be improved, any comments?
(the reason why I think performance will be improved is the image retrieval is almost random, so I do not think disk cache could work well in a random access pattern.)
regards,
George
|
|
|
|
|
Say i had this class:
public sealed class Partner
{
string something;
string somethingelse;
List<Customer> customers = new List<Customer>();
public List<Customer> Customers
{
get { return customers; }
}
} Is this bad practice? If so, how can i make it right and still be able to get all the methods a List<> provides? I know i could use class Partner : List<Customer> but inside Customer there's two Lists so that solution won't work there. Is it really necessary that i write a custom List for each object, just to maintain good coding practice? I'm asking because my teacher says that the above example is bad OOP - and because i'm developing an application that really needs the methods a List<> provides....
modified on Wednesday, November 26, 2008 8:22 AM
|
|
|
|
|
I think your generics got swallowed by the HTML monster!
xacc.ide - now with TabsToSpaces support IronScheme - 1.0 beta 1 - out now! ((lambda (x) `((lambda (x) ,x) ',x)) '`((lambda (x) ,x) ',x))
|
|
|
|
|
You're right - should be working now.
|
|
|
|
|
Depending on the usage, Customers could be IEnumerable<Customer> . But I do not see any other way of mutating that list.
Jan Sommer wrote: I'm asking because my teacher says that the above example is bad OOP
So did he not bother to explain to you why he feels that way? Sounds like a bad teacher to me. Ask him if he is happy with what he does
xacc.ide - now with TabsToSpaces support IronScheme - 1.0 beta 1 - out now! ((lambda (x) `((lambda (x) ,x) ',x)) '`((lambda (x) ,x) ',x))
|
|
|
|
|
He said he didn't know a better way :P
The first solution i came up with was to add this method to class Partner:
public IEnumerator<Customer> GetCustomersEnumerator()
{
return this.cust.GetEnumerator();
}
which is working just fine for retrieving all the customers with this code:
IEnumerator<Customer> alist = partners.GetCustomerEnumerator();
while(alist.MoveNext())
{
Console.WriteLine(alist.Current.someProperty);
}
But in the userinterface, when a user clicks the customer (shown in a listview) i'd like to use the customer-index to retrieve the correct customer and send him to another dialog. Currently i'm only able to access each customers information and i can't pick a specific customer, since i don't have access to the list.
Am i doing it all wrong? I was considering using a database which would make everything alot easier, but that would be too hard for the other students (according to my teacher :P)
|
|
|
|
|
Jan Sommer wrote: Am i doing it all wrong?
You have 2 choices:
1. Expose it as an IList<Customer> and do cust.ToArray() .
2. Wrap cust in a ReadOnlyCollection<Customer> .
I would prefer the latter.
xacc.ide - now with TabsToSpaces support IronScheme - 1.0 beta 1 - out now! ((lambda (x) `((lambda (x) ,x) ',x)) '`((lambda (x) ,x) ',x))
|
|
|
|
|
Thanks... Didn't know of the ReadOnlyCollection but that seems reasonable to use..
|
|
|
|
|
Why for heavens sake should this be bad coding practice??
This is the normal way to do it: Declaring a private field and exposing it through a public property. There's no other way to do it right - your teacher is definitely wrong!
There is one concern with this I can think of:
1. The list should be immutable. If so, simply use customers.AsReadOnly() in your get accessor and provide some methods like Add/RemoveCustomer in your Partner class.
Regards
Thomas
www.thomas-weller.de
Programming today is a race between software engineers striving to build bigger and better idiot-proof programs, and the Universe trying to produce bigger and better idiots. So far, the Universe is winning. Programmer - an organism that turns coffee into software.
|
|
|
|
|
I tend to agree. The only design guideline this might violate is from Bill Wagner's "Effective C#, Item 23: "Avoid Returning References to Internal Class Objects". But Thomas' suggestion to return a read-only object addresses (no pun intended) this issue.
|
|
|
|
|
i'm glad to hear i'm not completely wrong..
this sure have saved me a headache..
|
|
|
|
|
Always at your service...
Regards
Thomas
www.thomas-weller.de
Programming today is a race between software engineers striving to build bigger and better idiot-proof programs, and the Universe trying to produce bigger and better idiots. So far, the Universe is winning. Programmer - an organism that turns coffee into software.
|
|
|
|
|
Hello everyone,
I have a static class which wraps a file stream object. And I want to implement the Dispose pattern. I define an uninitialize method in the static class which call Dispose explicitly with parameter value true. Here is my code,
My questions are,
1. Normally there should be a destructor in a class which calls Dispose with parameter value false, but for a static class, there is no concept like destructor, how to implement calling Dispose with false parameter?
2. When the Dispose method without parameter will be called?
3. Is it correct to call GC.SuppressFinalize(this)? Since for a static class, there is no "this" object?
4. Correct to define disposed as static field?
5. Are there anything wrong with my code?
static private StreamWriter currentLogStream;
private static bool disposed = false;
public void Dispose()
{
Dispose(true);
GC.SuppressFinalize(this);
}
private static void Dispose(bool disposing)
{
if (false == disposed)
{
if (disposing)
{
currentLogStream.Dispose();
}
}
disposed = true;
}
public static void Uninitialize()
{
Dispose(true);
}
thanks in advance,
George
|
|
|
|
|
|
No, leppie! But Dispose(bool) is defined as static in my sample, how could I call it from destructor?
regards,
George
|
|
|
|
|
George_George wrote: But Dispose(bool) is defined as static in my sample, how could I call it from destructor?
If it is static, you cant! But why have it static? It does not really make sense to me
xacc.ide - now with TabsToSpaces support IronScheme - 1.0 beta 1 - out now! ((lambda (x) `((lambda (x) ,x) ',x)) '`((lambda (x) ,x) ',x))
|
|
|
|
|
Sorry, leppie! My mistake before. The class is singleton -- only once instance is created. Here is my whole code. Could you help to review whether my code is correct?
Thanks!
BTW: compile pass, seems we can call static method Dispose(bool) from destructor?
class Logger : IDisposable
{
private static bool disposed = false;
public void Dispose()
{
Dispose(true);
GC.SuppressFinalize(this);
}
~Logger()
{
Dispose(false);
}
private static void Dispose(bool disposing)
{
if (false == disposed)
{
if (disposing)
{
currentLogStream.Dispose();
}
}
disposed = true;
}
public static void Uninitialize()
{
Dispose(true);
}
}
regards,
George
|
|
|
|
|
George_George wrote: he class is singleton -- only once instance is created.
So you still have an instance, use it! I think you are abusing the singleton pattern a bit
xacc.ide - now with TabsToSpaces support IronScheme - 1.0 beta 1 - out now! ((lambda (x) `((lambda (x) ,x) ',x)) '`((lambda (x) ,x) ',x))
|
|
|
|
|
|
It still looks fishy to me
Make all those 'static' members, private 'instance' member. As your instance will be a singleton, any instance members of it will also be a singleton (conceptually).
xacc.ide - now with TabsToSpaces support IronScheme - 1.0 beta 1 - out now! ((lambda (x) `((lambda (x) ,x) ',x)) '`((lambda (x) ,x) ',x))
|
|
|
|
|
Thanks leppie,
Could you show me your code please, which you think is better than my current code?
regards,
George
|
|
|
|
|
When you say "destructor" are you talking about writing a finalizer (~ClassName) method? If so, then you generally don't need to do this. Just because you implement Dispose does not mean you need to implement a finalizer.
The Dispose() method is called by another programmer when they are done using your object.
Yes, you normally would call GC.SuppressFinalizer(this) in your Dispose method. In the case of a static class there is no "this" object. Normally static classes do not hold any internal state and would not actually need to do any object disposal of their own. In your case, since this is a singleton class you will have some internal state however you are implementing the dispose pattern because you are managing another disposable class. In this case you actually don't need to implement the full dispose pattern and can get away with doing everything in the simple Dispose() method.
Again, normally the disposed field is an instance field as the pattern really only applies to non-static classes.
There are a few things wrong, ignoring the fact that this is a static class. If it were not a static class, it should be implmented as:
public class StreamWrapper : IDisposable
{
private StreamWriter currentLogStream;
private bool disposed;
public void Dispose()
{
this.Uninitialize();
}
protected virtual void Dispose(bool disposing)
{
if (!disposed)
{
if (disposing)
{
if (currentLogStream != null)
{
currentLogStream.Dispose();
}
}
}
disposed = true;
}
public void Uninitalize()
{
Dispose(true);
GC.SuppressFinalize(this);
}
} Take a look at this article[^] for more details on how to implement dispose properly.
Scott Dorman Microsoft® MVP - Visual C# | MCPD
President - Tampa Bay IASA
[ Blog][ Articles][ Forum Guidelines] Hey, hey, hey. Don't be mean. We don't have to be mean because, remember, no matter where you go, there you are. - Buckaroo Banzai
|
|
|
|
|
Thanks Scott,
1.
"If so, then you generally don't need to do this" -- confused. Do you mean there is no need to call Dispose method in finalizer? If so, I disagree and in MSDN Dispose pattern, in finalizer dispose method is called with false value parameter.
2.
You mentioned "Normally static classes do not hold any internal state and would not actually need to do any object disposal of their own." -- confused. It is common to have a static class hold a member variable which pointed to disposable object instance, like stream. In your points, there is no need to dispose the wrapped disposable object instance?
3.
In my situation, the initialize method has to be static.
I have fixed some issues you mentioned and posted my new code below. Could you help to review whether my code is correct? Any potential issues?
class Logger : IDisposable
{
private static bool disposed = false;
private static StreamWriter logStream;
public void Dispose()
{
Dispose(true);
GC.SuppressFinalize(this);
}
~Logger()
{
Dispose(false);
}
private static void Dispose(bool disposing)
{
if (false == disposed)
{
if (disposing)
{
LogStream.Dispose();
LogStream = null;
}
}
disposed = true;
}
public static void Uninitialize()
{
Dispose(true);
GC.SuppressFinalize(l ogStream);
}
}
regards,
George
|
|
|
|
|
1. I am talking about writing a finalizer. In your class this would be a method named ~Logger(). There is generally no need to write a finalizer, however you should always implement IDisposable if your object maintains unmanaged resources, derives from something that implements IDisposable, or otherwise maintains disposable objects.
2. You are confused with the difference between a static class and a singleton. A singleton is a design pattern that ensures there is ever only one instance of the class. Singletons are not static classes. (Your code also does not implement a static class either...simply a "normal" class with static members. There is a difference.)
3. Again, you are confusing a static class and a singleton. In this scenario, you need to forget about using static anything.
Here is a more complete example of using a singleton:
public sealed class Logger : IDisposable
{
private static Logger instance;
private StreamWriter logStream;
private bool disposed;
private static readonly object syncLock = new object();
private Logger()
{
}
public void Dispose()
{
this.Uninitialize();
}
protected virtual void Dispose(bool disposing)
{
if (!disposed)
{
if (disposing)
{
if (logStream != null)
{
logStream.Dispose();
logStream = null;
}
}
}
disposed = true;
}
public void Uninitalize()
{
Dispose(true);
GC.SuppressFinalize(this);
}
public static Logger Instance
{
get
{
lock(syncLock)
{
if (instance == null)
{
instance = new Logger();
}
}
return instance;
}
}
} This shows how you would use the singleton object. The actual class is sealed with a private constructor, so the only way to get it to do anything meaningful is to access it through the static Instance property. This will return a new instance of the Logger class if one hasn't already been created; if one has been created it will return that instance
Logger.Instance.Uninitalize();
Scott Dorman Microsoft® MVP - Visual C# | MCPD
President - Tampa Bay IASA
[ Blog][ Articles][ Forum Guidelines] Hey, hey, hey. Don't be mean. We don't have to be mean because, remember, no matter where you go, there you are. - Buckaroo Banzai
|
|
|
|
|