Which is more correct?
-
Example 1.
public class Employees
{
public static List ListEmployees;public static void FillEmployees() { ListEmployees = FillFromSQL(); }}
class Program
{
static void Main(string[] args)
{
Employees.FillEmployees();
}
}Then whenever I need to access the list of employee objects I can just call Employees.ListEmployees.Find(e => e.Name == "Mike"); I can call this anywhere in my scope because is static. Or Example 2
public class Employees
{
private List ListEmployees;public void FillEmployees() { ListEmployees = FillFromSQL(); } public void FindEmployee(Predicate e) { return ListEmployees.Find(e); }}
class Program
{
Employees employees;static void Main(string\[\] args) { employees = new Employees(); employees.FillEmployees(); }}
Any help would be much appreciated. Or if you have a better way of doing it that would be a great help.
-
I don't know the entire context of your program of course, but I think I'd use a
Dictionary<string, Employee>(where thestringis the name)Employees can have the same name though. I do see what you saying, but problem lies more on the way I build my classes. Having most of the list static so I can access them from anywhere. I don't think this is right though. Well it is right but proberly doesn't follow best practises.
-
Example 1.
public class Employees
{
public static List ListEmployees;public static void FillEmployees() { ListEmployees = FillFromSQL(); }}
class Program
{
static void Main(string[] args)
{
Employees.FillEmployees();
}
}Then whenever I need to access the list of employee objects I can just call Employees.ListEmployees.Find(e => e.Name == "Mike"); I can call this anywhere in my scope because is static. Or Example 2
public class Employees
{
private List ListEmployees;public void FillEmployees() { ListEmployees = FillFromSQL(); } public void FindEmployee(Predicate e) { return ListEmployees.Find(e); }}
class Program
{
Employees employees;static void Main(string\[\] args) { employees = new Employees(); employees.FillEmployees(); }}
Any help would be much appreciated. Or if you have a better way of doing it that would be a great help.
I would do this:
public class Employee
{
}public class EmployeeList : List
{
public EmployeeList(bool populateNow)
{
if (populateNow)
{
GetFromDatabase();
}
}public void GetFromDatabase() { // do whatever you need to do to retrieve the employees from the database this.AddRange(blah blah); }}
EmployeeList employees = new EmployeeList(true);
The list should know how to load itself, and can take care of all the nasty stuff behind the scenes and abstracted out of the UI code.
.45 ACP - because shooting twice is just silly
-----
"Why don't you tie a kerosene-soaked rag around your ankles so the ants won't climb up and eat your candy ass..." - Dale Earnhardt, 1997
-----
"The staggering layers of obscenity in your statement make it a work of art on so many levels." - J. Jystad, 2001 -
Example 1.
public class Employees
{
public static List ListEmployees;public static void FillEmployees() { ListEmployees = FillFromSQL(); }}
class Program
{
static void Main(string[] args)
{
Employees.FillEmployees();
}
}Then whenever I need to access the list of employee objects I can just call Employees.ListEmployees.Find(e => e.Name == "Mike"); I can call this anywhere in my scope because is static. Or Example 2
public class Employees
{
private List ListEmployees;public void FillEmployees() { ListEmployees = FillFromSQL(); } public void FindEmployee(Predicate e) { return ListEmployees.Find(e); }}
class Program
{
Employees employees;static void Main(string\[\] args) { employees = new Employees(); employees.FillEmployees(); }}
Any help would be much appreciated. Or if you have a better way of doing it that would be a great help.
If you want to cache your Employees for use throughout your application and it's acceptable to not reload them (i.e. they will not be changed by someone else acessing the DB) then it's OK to keep a static reference. One or two points though. Don't make fields public, you should create a static readonly property (i.e. only a getter) to expose the cached List. If you have a lot of data stored in your list then you may end up with memory issues, so it may be wise to only store the basic information such as ID and Name, and look up the rest of the data when required. Have you considered using the Singleton pattern and have a Manager/Factory class that creates the list only when it is first needed? It won't offer any huge improvements, but can be a more logical approach. Without knowing the exact requirements and other implementation details it's hard to say but may be worth investigating.
Dave
BTW, in software, hope and pray is not a viable strategy. (Luc Pattyn)
Why are you using VB6? Do you hate yourself? (Christian Graus) -
I would do this:
public class Employee
{
}public class EmployeeList : List
{
public EmployeeList(bool populateNow)
{
if (populateNow)
{
GetFromDatabase();
}
}public void GetFromDatabase() { // do whatever you need to do to retrieve the employees from the database this.AddRange(blah blah); }}
EmployeeList employees = new EmployeeList(true);
The list should know how to load itself, and can take care of all the nasty stuff behind the scenes and abstracted out of the UI code.
.45 ACP - because shooting twice is just silly
-----
"Why don't you tie a kerosene-soaked rag around your ankles so the ants won't climb up and eat your candy ass..." - Dale Earnhardt, 1997
-----
"The staggering layers of obscenity in your statement make it a work of art on so many levels." - J. Jystad, 2001 -
If you want to cache your Employees for use throughout your application and it's acceptable to not reload them (i.e. they will not be changed by someone else acessing the DB) then it's OK to keep a static reference. One or two points though. Don't make fields public, you should create a static readonly property (i.e. only a getter) to expose the cached List. If you have a lot of data stored in your list then you may end up with memory issues, so it may be wise to only store the basic information such as ID and Name, and look up the rest of the data when required. Have you considered using the Singleton pattern and have a Manager/Factory class that creates the list only when it is first needed? It won't offer any huge improvements, but can be a more logical approach. Without knowing the exact requirements and other implementation details it's hard to say but may be worth investigating.
Dave
BTW, in software, hope and pray is not a viable strategy. (Luc Pattyn)
Why are you using VB6? Do you hate yourself? (Christian Graus)Thanks, I do have quite a few records, anywhere up to 10000. I do quite a few checks on the List though, every day I get a bulk file of employees and check if the employee exists, if he does I need to check if any of his details have changed. (Every single field) I think one select of all the data and then data lookups would be faster then 10000 selects?
-
Thanks, I do have quite a few records, anywhere up to 10000. I do quite a few checks on the List though, every day I get a bulk file of employees and check if the employee exists, if he does I need to check if any of his details have changed. (Every single field) I think one select of all the data and then data lookups would be faster then 10000 selects?
10000 doesn't sound like a lot, it depends on how much space each element takes. If it's only say 100 bytes then that's approx 1MB of memory. If however you have names, addresses, photo etc... each record could be much larger. If it were a meg each then that would be 10GB and unless you have a mega system you would come crashing to a halt. Even if useage is low, the volume of Employees may change, and the storage requirements may expand later (scans of signed contracts of employemt as an example) so I would build for that now and just cache the data that is required to make the application useable, and do a DB lookup for the non cached data when needed.
Dave
BTW, in software, hope and pray is not a viable strategy. (Luc Pattyn)
Why are you using VB6? Do you hate yourself? (Christian Graus) -
10000 doesn't sound like a lot, it depends on how much space each element takes. If it's only say 100 bytes then that's approx 1MB of memory. If however you have names, addresses, photo etc... each record could be much larger. If it were a meg each then that would be 10GB and unless you have a mega system you would come crashing to a halt. Even if useage is low, the volume of Employees may change, and the storage requirements may expand later (scans of signed contracts of employemt as an example) so I would build for that now and just cache the data that is required to make the application useable, and do a DB lookup for the non cached data when needed.
Dave
BTW, in software, hope and pray is not a viable strategy. (Luc Pattyn)
Why are you using VB6? Do you hate yourself? (Christian Graus) -
Employees can have the same name though. I do see what you saying, but problem lies more on the way I build my classes. Having most of the list static so I can access them from anywhere. I don't think this is right though. Well it is right but proberly doesn't follow best practises.
Bardy85 wrote:
Well it is right but proberly doesn't follow best practises.
A "best practice" often turns into a golden hammer. There's a lot of ways to load data, some make sense in a webapp, others make more sense in a console-application. Using NHibernate from a console-app just to update the name of a single employee would be overkill. Most answers here will be variations on the ActiveRecord-pattern to store a single record. It would also make sense to store multiple records in some kind of collection[^].
I are Troll :suss:
-
Example 1.
public class Employees
{
public static List ListEmployees;public static void FillEmployees() { ListEmployees = FillFromSQL(); }}
class Program
{
static void Main(string[] args)
{
Employees.FillEmployees();
}
}Then whenever I need to access the list of employee objects I can just call Employees.ListEmployees.Find(e => e.Name == "Mike"); I can call this anywhere in my scope because is static. Or Example 2
public class Employees
{
private List ListEmployees;public void FillEmployees() { ListEmployees = FillFromSQL(); } public void FindEmployee(Predicate e) { return ListEmployees.Find(e); }}
class Program
{
Employees employees;static void Main(string\[\] args) { employees = new Employees(); employees.FillEmployees(); }}
Any help would be much appreciated. Or if you have a better way of doing it that would be a great help.
Hi, 1. if it is a list, then make it inherit from List, as John said. 2. static seldom is good; what happens if you suddenly need two lists of employees, e.g. because your program suddenly has to deal with two companies? the best use for static is for holding overall counters and other statistical information on your class, information that transcends all the class instances. :)
Luc Pattyn [Forum Guidelines] [Why QA sucks] [My Articles]
I only read code that is properly formatted, adding PRE tags is the easiest way to obtain that.
modified on Sunday, January 17, 2010 5:07 PM
-
Hi, 1. if it is a list, then make it inherit from List, as John said. 2. static seldom is good; what happens if you suddenly need two lists of employees, e.g. because your program suddenly has to deal with two companies? the best use for static is for holding overall counters and other statistical information on your class, information that transcends all the class instances. :)
Luc Pattyn [Forum Guidelines] [Why QA sucks] [My Articles]
I only read code that is properly formatted, adding PRE tags is the easiest way to obtain that.
modified on Sunday, January 17, 2010 5:07 PM
-
Thanks, I've just started implementing the : List. Don't know why I didn't think about it before. Thanks again for the help. :)
You're welcome. :)
Luc Pattyn [Forum Guidelines] [Why QA sucks] [My Articles]
I only read code that is properly formatted, adding PRE tags is the easiest way to obtain that.
-
Thanks, nice way of doing it. If you wanted to initialize the list once at start of your application and then be able to call it through out the scope of you app, how would you do it?
Put it in a static class and intitialize it in the constructor of the that class.
public static class Globals
{
public static EmployeeList Employees { get; private set; }static Globals() { Employees = new EmployeeList(true); }}
.45 ACP - because shooting twice is just silly
-----
"Why don't you tie a kerosene-soaked rag around your ankles so the ants won't climb up and eat your candy ass..." - Dale Earnhardt, 1997
-----
"The staggering layers of obscenity in your statement make it a work of art on so many levels." - J. Jystad, 2001
