Following code is supposed generate a new ID (PID) for a new client based on the client name's first letter and the existing IDs in the range which are stored in a database.
For ex lets say if the new client's name is "Abc" and if last ID in the database starting from letter A, is A200. Then the new ID should be A201.
This code compiles and runs giving the desired out put but I hope this could be optimized further.
Expecting your suggestions on improving this...
private void GeneratePIDforNewPoint(string pointName)
{
string firstLetterOfPointName; // variable to hold first letter of "point name" being passed to this method
string lastExistingPID; // variable to hold the last PID (most recent hence largest) from the list of PIDs retrieved from the database
int lastDigitOfExistingPID; // variable to hold last digit of the "lastPID" (ex- digit '2' from PID 'F002')
string finalPID; // variable to hold out put (the generated new PID for the new point)...
firstLetterOfPointName = pointName[0]; // Get the fist letter of the point name...
List<string> pidList=_ds.GetPIDs(firstLetterOfPointName); // Calling method to get the pid list from database
if (pidList.Count >=1) // At the end of this if block, numeric part of the new PID will be desided
{
pidList.Sort();
lastExistingPID = pidList[pidList.Count - 1];
lastDigitOfExistingPID = int.Parse(lastExistingPID[lastExistingPID.Length - 1].ToString());
}
else
{
lastDigitOfExistingPID = 0;
}
lastDigitOfExistingPID += 1;
// Found digit will be converted to the pid format by attaching the starting letter and zeros to make the length.(format --> A001/ B099 / C100 etc.)
finalPID= firstLetterOfPointName + String.Format("{0:000}", lastDigitOfExistingPID );
_view.PID = finalPID;
}
public List<string> GetPIDs(string firstLetterOfPointName)
{
string selectStatement = @"SELECT PID FROM point WHERE PID like @PID";
List<string> pidList = new List<string>(); // List to store all retrived PIDs from the database...
// Retrieve all existing PIDs starting with "letter" from the database
using (SqlConnection sqlConnection = new SqlConnection(db.GetConnectionString))
{
using (SqlCommand sqlCommand = new SqlCommand(selectStatement, sqlConnection))
{
sqlCommand.Parameters.Add("@PID", SqlDbType.VarChar).Value = firstLetterOfPointName + '%';
sqlConnection.Open();
using (SqlDataReader dataReader = sqlCommand.ExecuteReader())
{
// If reader has data, are added to the list
while (dataReader.Read())
{
pidList.Add(dataReader.GetString(0).Trim());
}
return pidList;
}
}
}
}
2 Answers 2
First, I would expect from a function named GeneratePID to return the generated ID and not to set it. (Nobody likes side-effects)
private string GeneratePIDforNewPoint(string pointName)
{
if(pointName == null)
return "error";
var storedPIDs = _ds.GetPIDs(pointName[0]);
var newPID = 0;
if(storedPIDs.Count > 0) {
var maximumStoredPID = _ds.GetPIDs(pointName[0]).Max();
newPID = Int32.Parse(maximumStoredPID.substring(1)) + 1;
}
return pointName[0] + String.Format("{0:000}", newPID);
}
-
\$\begingroup\$ Lack of side effects destroys the utility. Unless this code is single-threaded and non-reentrant, somehow you have to increment a count and/or store the new ID. \$\endgroup\$Bob Dalgleish– Bob Dalgleish2014年04月30日 00:38:52 +00:00Commented Apr 30, 2014 at 0:38
-
\$\begingroup\$ I think when using the count, problem might be, if a record deleted accidentally in the table? Ex: Lets say it had A001,A002 and A003. Now A002 was deleted. The above code will generate A003 as new PID and would raise an error( violation of key constrain). How to avoid this? \$\endgroup\$CAD– CAD2014年04月30日 03:26:51 +00:00Commented Apr 30, 2014 at 3:26
-
1\$\begingroup\$ Okay, in that case you have to find the maximum PID. I've edited the code above. \$\endgroup\$ceco– ceco2014年04月30日 07:02:41 +00:00Commented Apr 30, 2014 at 7:02
It's been an old question, just thought to add an alternative to a name-with-counter solution already offered above.
ID using CRC32
ID based on data
Usually when I need an ID that is based on user's full name or some other type of data, I'd do something like this:
php example
class myUtils {
static public function genID( $firstName, $lastName ) {
$strDesc = $firstName . "-" . $lastName;
return "".dechex(crc32($strDesc));
}
}
This will return an 8-character long string representation of a 32-bit integer, for example: a072c35b
This is then simple and fast to use:
$id = myUtils::genID("First name","Last name");
This way you can generate ID that represents bunch of data grouped together, for example instead of just providing first and last name, you can add his passport number into the mix etc. to generate ID that is unique to that user.
However, if you just require a unique ID number, there's even simpler way:
Just a Unique ID
If I need to generate a unique ID that is just that - unique, I'd go for something like this:
php example
class myUtils {
static public function uid_32bit() {
return "".dechex(crc32(date("YdmHis").bin2hex(openssl_random_pseudo_bytes(10))));
}
static public function uid_64bit() {
return self::uid_32bit().self::uid_32bit();
}
}
This way I could get 8-character long unique string - representing 32-bit integer:
$id = myUtils::uid_32bit();
or, I could get 16-character long unique string, representing 64-bit integer value:
$id = myUtils::uid_64bit();
Main advantage of this approach is that is easily read by humans, as it contains only lowercase letters (a...f) and numbers (0...9), and can be used for instance invoice unique numbers that are shown to user, or some other client-facing output, without revealing any information about how you have structured your data in the back-end.
-
\$\begingroup\$ Why do you use
date($strDesc)
like this? What does it do? (I believe I know why, but I'd like to hear your reasoning.) \$\endgroup\$RoToRa– RoToRa2019年10月18日 08:04:44 +00:00Commented Oct 18, 2019 at 8:04 -
\$\begingroup\$ Ahhh crap, that was copy-paste error. I copied function from the 2nd code snippet that uses date, but missed to remove date part. In the first code snippet It's supposed to calculate crc32 from generated string. I've edited the code snippet to fix this bug. Thanks! \$\endgroup\$Siniša– Siniša2019年10月20日 11:37:58 +00:00Commented Oct 20, 2019 at 11:37
create table charid { id char, count long }
. Search forpointName[0]
asid
in the table, incrementcount
and write it back while locking the row. IfpointName[0]
is not in the table, add it and set thecount
to 1 while locking the table. \$\endgroup\$