Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

This is the situation : the table order has a column with ID AUTO INCREMENT but (there is always a but)
i can't use it because i don't want number order 1234567 after 5 years but i want 1/2014 , 1/2015 etc etc. For doing this i have created this columns into the table :

ID AUTO INCREMENT <--- unique ID used for manage the order after the insert  
numberorder INT <--- fake number of order showed at the user NOT unique  
IDyear YEAR <--- the year for the order e.g. 2014 or 2015

For doing this i write this code before the query for insert a new order
and processed at the submit form :

$query_Recordset4 = "SELECT MAX(IDyear) AS maxyear FROM order";
$Recordset4 = $mysqli->query($query_Recordset4);
$row_Recordset4 = $Recordset4->fetch_assoc();
$maxyear = $row_Recordset4['maxyear'] ;
$thisyear = date("Y") ;

if ($maxyear === $thisyear) {
    $query_Recordset5 = "SELECT MAX(numberorder) FROM order WHERE IDyear=$thisyear ;";
    $Recordset5 = $mysqli->query($query_Recordset5);
    $row_Recordset5 = $Recordset5->fetch_assoc();
    $numberorder = $row_Recordset5['MAX(numberorder)'] + 1 ;
}
else {  
    $numberorder = 1 ;
}

And then i use $numberorder for insert them into the table order in his column.

QUESTION REVIEW

Maybe my question was not clear written and now i try to add some elements.

Thit is what i have now into the table order :

|----------|-------------|--------|
|    ID    | numberorder | yearId |
|----------|-------------|--------|
!    1     |     1       |  2014  |
|----------|-------------|--------|
|    "     |     "       |    "   |  <---- more orders
|----------|-------------|--------|
|   500    |    500      |  2014  |  <---- last order year 2014
|----------|-------------|--------|

When the year change what i need is this :

|----------|-------------|--------|
|   501    |     1       |  2015  |  <---- first order year 2015 (2016,2017 etc etc)
|----------|-------------|--------|

Like a suggestion, my code has a concurrency problem.

Have a second table yearid with another AUTO_INCREMENT field for take to them a unique ID for numberorder field is perfect solution but when the year finish i can't restarting the AUTO_INCREMENT at 1 because for security reason the TRUNCATE and DROP commands are not available for the user created for the connection.

The only way i see is create a new table each year e.g. yearid2015 for have ID = 1.

share|improve this question
    
It would be more helpful if you included your database table schema. –  Simon André Forsberg Mar 1 at 18:02
    
@SimonAndréForsberg done :) –  geomo Mar 1 at 18:45
add comment

2 Answers 2

$query_Recordset4 = "SELECT MAX(IDyear) AS maxyear FROM order";

Whenever you need to postfix the name of a variable, class or anything else with number because you already used that name...you're doing it wrong. Either your function does too much and should be split into smaller functions, or you need to choose better names.


$query_Recordset5 = "SELECT MAX(numberorder) FROM order WHERE IDyear=$thisyear ;";

Even when you declare the variable, you should make a habit out of using parameterized queries.


There's something wrong about your table layout. At the moment I imagine it should look something like this:

TABLE order
    numberorder INT
    IDyear INT

A more sane layout would be this:

TABLE order
    id INT AUTO_INCREMENT
    idYear INT
    orderedAt DATETIME

The main difference is that I rely on an ID field which is managed by the database. It does not matter what the accountant wants or how your boss thinks the billing numbers should look like, you need something unique to identify a record and nothing beats a unique, automatically managed counter for doing that job.

Creating the necessary number you want is as easy as

SELECT CONCAT(idYear, '/', YEAR(orderedAt)) FROM order;

and if you place this in a stored procedure or in a function in your object you never need to think about it again. Incrementing this number is also rather easy:

SELECT IFNULL(MAX(idYear), 0) + 1 FROM order WHERE YEAR(orderedAt) = yourYear;

It can not break and you're free to put the date of the order into the past or the future (which is sometimes needed in accounting). In your method working outside of the current year will break the numbering.


Ideally you would set that number if you insert the record into the database to minimize race-conditions (and possible duplicate numbers).

share|improve this answer
    
My method is "for break the numbering" and restart at 1 each year. The first column in order table is ID AUTO INCREMNT and for the management of the order i refer at this unique ID but at the user is showed only the "fake" id nuber $numberorder e.g. 12 / 2014 where 12 is $numberorder and 2014 is IDyear (ok, ok.. now i change it in idYear ;) ) –  geomo Mar 1 at 17:35
add comment

This is not so much a question about mysqli and php, but about databases and database theory.

Consider two simultaneous accesses to your page... they will both get the same result for:

$numberorder = $row_Recordset5['MAX(numberorder)'] + 1 ;

and they will both try to insert with the same ID value, and one of them will fail.

The AUTOINCREMENT system is designed to be concurrently safe, and guarantees that just one record has the value.

Depending on who you read/listen/pay attention to, you will get different answers, but, in your case, you really have to question: Why do you need a special primary key?. If the answer is because you want a value that fits some other format, well, then, there are other solutions.... for example, you can do the following:

create table YearId (
     int year not null,
     int firstid not null,
     int lastid not null
}

Then you can have a column that you can update as :

update table set yearid = (select year + '/' + (id - firstid) from YearId where id between firstid and lastid)

The alternatives to this sort of logic are complicated transactions that need to be managed in PHP using begin-transactions and commit-transactions.

Doing that would be a 'leap' in your requirements.

share|improve this answer
    
the code work when the user click on submit button, not before and bring ofcourse the last ID. No way to have the same ID value. –  geomo Mar 1 at 17:29
    
@geomo - what if two people click the submit button at the same time...? –  rolfl Mar 1 at 17:32
    
sorry for the lag.. we have try now many time.. always one user arrived first then other.. even if we click at the same time.. i think only two robot doing this but if the line cable is more long then other so one query arrived second. –  geomo Mar 1 at 17:57
1  
@geomo Trying to check if concurrency works or not by trying to click a button "at the same time" isn't very useful. I think rolfl is right here and that it could cause a problem. If that problem happens once every 10 times, once every 1000 times or once every million times is another question - but it is still a problem. Using rolfl's approach, you can guarantee that there will not be a problem. –  Simon André Forsberg Mar 1 at 18:01
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.