PHP programmer
Discussion
Well that's all well & good but finding a developer that will sit down, look at it & make the changes required seems the hard bit here. At the end of the day I paid a PH'er to make a cart system which they did do, it all works fine but for some reason the 3Ds part started passing the incorrect amounts. I know for a fact it did all work 100% so maybe it was a PHP upgrade or apache upgrade that broke it. Finding the person that wrote this isn't really the issue, they are active on here but I'm not here for that. The cart passes PCI scanning & works fine. If you know someone who would look at it & go through point by point what should change & why then fire away.
Sorry but that's like secondary school level coding, at best.
As said already there should be no formatting of numbers for calculation, either one way or the other. I have no idea why those numbers are being multiplied by 100, either, at least going by the assumed purpose of the variables from their name.
Of what little you've posted I'm scared about what else lies beneath. I'd ask you to post a link to your website but it's maybe best that you don't.
EDIT: How much code is there on the website? Was the entire thing written from scratch? I don't mind taking a look but can't easily look at it until later, maybe even tomorrow.
As said already there should be no formatting of numbers for calculation, either one way or the other. I have no idea why those numbers are being multiplied by 100, either, at least going by the assumed purpose of the variables from their name.
Of what little you've posted I'm scared about what else lies beneath. I'd ask you to post a link to your website but it's maybe best that you don't.
EDIT: How much code is there on the website? Was the entire thing written from scratch? I don't mind taking a look but can't easily look at it until later, maybe even tomorrow.
Edited by Durzel on Monday 8th February 18:51
Well that's a shame. I paid someone to do I job and as far as I was aware they'd done it (apart from the parts that were never finished which I've since done with help & by working through it myself). This is the trouble with this stuff, you get someone to do it then someone else looks at it & tells you it's utter s
t & start again! It's not always viable to do that. It's easy to say from a coders POV but when that means the business basically starting from scratch it's not always possible. I look at big sites like Royal Mail & they have bugs that have not been fixed for months so this kind of thing affects big & small. I'll not mention the bugs Pay Pal have that have been awaiting a fix for 4 months now too! (which they know about but seem unable to fix!)
t & start again! It's not always viable to do that. It's easy to say from a coders POV but when that means the business basically starting from scratch it's not always possible. I look at big sites like Royal Mail & they have bugs that have not been fixed for months so this kind of thing affects big & small. I'll not mention the bugs Pay Pal have that have been awaiting a fix for 4 months now too! (which they know about but seem unable to fix!)Durzel said:
How much code is there on the website? Was the entire thing written from scratch? I don't mind taking a look but can't easily look at it until later, maybe even tomorrow.
Yep, AFAIK it was written 100% from scratch using code examples from the 3DS processor & Pay Pal. Back then we also had Google Checkout as a gateway but that has been disabled for years as they closed it down. I honestly think the person who wrote it knew what they were doing (despite some of the strong comments on this post!!), I'm certainly not here to start chucking their name around as I feel even though they didn't finish the project (& hence I never finished paying the entire fee!) that would be pretty unfair.Edited by Dave_ST220 on Monday 8th February 19:00
Durzel said:
As said already there should be no formatting of numbers for calculation, either one way or the other. I have no idea why those numbers are being multiplied by 100, either, at least going by the assumed purpose of the variables from their name.
Quite often people use integers to pass round currency values to help avoid rounding errors and this is a very naive attempt at a conversion. Many programming languages have special types to deal with this. Both Ruby and Java have a BigDecimal class.
http://ruby-doc.org/stdlib-2.1.1/libdoc/bigdecimal...
https://docs.oracle.com/javase/7/docs/api/java/mat...
However you still need to be careful, especially when dealing with Tax as some calculations are rounded up and some are rounded down.
This bit here
is formatting the VAT amount to two decimal places and adding comma-separators for thousands, so it might be easier to find other bits of code that format the other numbers in similar ways and remove them, than go through stripping out commas. Unless you've got it all working, in which case stick with it. The fact that the start of your code references $row[0], $row[1] and so on suggest that these are being retrieved from the database which would mean that they are being stored in a formatted way, which doesn't sound good.
Have a look at the functions used to access the database as well - if they look like this, then you may have another issue any time soon: mysql_query("some text") or mysql_close(). Functions that use the old-style calls (as opposed to mysqli or PDO) won't work when your host upgrades to PHP7.
$vatamount = number_format($vatamount, 2, '.', ',');
is formatting the VAT amount to two decimal places and adding comma-separators for thousands, so it might be easier to find other bits of code that format the other numbers in similar ways and remove them, than go through stripping out commas. Unless you've got it all working, in which case stick with it. The fact that the start of your code references $row[0], $row[1] and so on suggest that these are being retrieved from the database which would mean that they are being stored in a formatted way, which doesn't sound good.
Have a look at the functions used to access the database as well - if they look like this, then you may have another issue any time soon: mysql_query("some text") or mysql_close(). Functions that use the old-style calls (as opposed to mysqli or PDO) won't work when your host upgrades to PHP7.
droopsnoot said:
The fact that the start of your code references $row[0], $row[1] and so on suggest that these are being retrieved from the database which would mean that they are being stored in a formatted way, which doesn't sound good.
Good spot - they should be stored either has bigInt or Decimal in the database. If values are being stored as strings in the Database this is a fundamental error which needs fixing.droopsnoot said:
This bit here
is formatting the VAT amount to two decimal places and adding comma-separators for thousands, so it might be easier to find other bits of code that format the other numbers in similar ways and remove them, than go through stripping out commas. Unless you've got it all working, in which case stick with it. The fact that the start of your code references $row[0], $row[1] and so on suggest that these are being retrieved from the database which would mean that they are being stored in a formatted way, which doesn't sound good.
Have a look at the functions used to access the database as well - if they look like this, then you may have another issue any time soon: mysql_query("some text") or mysql_close(). Functions that use the old-style calls (as opposed to mysqli or PDO) won't work when your host upgrades to PHP7.
It is woking now. Re: your point about php 7, what's the fix there?! Is it just a case of replacing old commands with new or is it a big issue?! $vatamount = number_format($vatamount, 2, '.', ',');
is formatting the VAT amount to two decimal places and adding comma-separators for thousands, so it might be easier to find other bits of code that format the other numbers in similar ways and remove them, than go through stripping out commas. Unless you've got it all working, in which case stick with it. The fact that the start of your code references $row[0], $row[1] and so on suggest that these are being retrieved from the database which would mean that they are being stored in a formatted way, which doesn't sound good.
Have a look at the functions used to access the database as well - if they look like this, then you may have another issue any time soon: mysql_query("some text") or mysql_close(). Functions that use the old-style calls (as opposed to mysqli or PDO) won't work when your host upgrades to PHP7.
woowahwoo said:
Although, I am not averse to the DIY approach, it seems you are 'hacking' at crucial aspects of a system that is important to you without the basis of fundamental knowledge to go with it. Be careful!!! 
Thanks. I back up anything I change & test it. At the moment I don't have much choice. The support staff from the processor are zero help. I guess this is the pitfall of having something custom written rather than an off the shelf package. Live & learn. I'm not buying that other people's money shouldn't be touching this code though. At the end if the day it passes PCI scanning and takes the payment securely. We aren't just billing people random amounts!! Pay Pal is riddled with bugs, how many payments are they dealing with?! Ironically the developer who did this work actually highlighted bugs in their new system which after months they admitted to! He wasn't all bad!!!woowahwoo said:
Without seeing all of the code, it's difficult to say what is going on and those decrying it speak from a position of ignorance. Not least because they don't know what you specified and what you paid for it. Some like to think they live in a land where their code is built to strict, test and specification-conforming, engineering-type standards (managers/directors are often keen to appear incredulous at finding this is not the case when the sts hits the fans). But, as is shown week in week out with breach after breach, and as you yourself identify, most organisations, even those sufficiently large enough to employ at least two people to work on this sort of thing, frequently produce, at best, prototypal test systems gone-live, and at worst fragile, insecure garbage timebombs built over time by a revolving door of either gross ineptitude or the demands of short-cutting.
Whilst I agree with the central point you make, and that 10 people looking at the code/requirements would likely produce 10 different solutions, based on what has been pasted alone it's fairly clear that there has been a basic lack of understanding of data handling, etc. ts hits the fans). But, as is shown week in week out with breach after breach, and as you yourself identify, most organisations, even those sufficiently large enough to employ at least two people to work on this sort of thing, frequently produce, at best, prototypal test systems gone-live, and at worst fragile, insecure garbage timebombs built over time by a revolving door of either gross ineptitude or the demands of short-cutting.That said, as you rightly point out, no one knows how much was charged or how long the guy had to do it.
Edit: I'm not a huge fan of PayPal but unless you're doing some really esoteric stuff with them, which I can almost guarantee you are not, then their core payment systems are robust and well documented, IME anyway.
Edited by Durzel on Monday 8th February 20:14
What you're referring to is TDD, test driven development. Most (half-decent) dev teams do this and more already but it's much harder for small companies to do such a thing, they generally do as the OP has, there's no other choice really.
It's not "old style code" though, it's a clear indicator of not appreciating the difference between the storage and presentation layers.
It's not "old style code" though, it's a clear indicator of not appreciating the difference between the storage and presentation layers.
Yes, but part of those tests will include automating the testing of front-end user requirements using something like Selenium, assuming you get them from the user.
No amount of testing would highlight the issue with the number/string mangling on the server side though, that would only come up in a code review. The OP had none of those options.
No amount of testing would highlight the issue with the number/string mangling on the server side though, that would only come up in a code review. The OP had none of those options.
woowahwoo said:
Disagree. As I said, the OP could, quite easily, have tested the basket/cart at various order values and quantities, spamming the inputs with garbage etc... No selenium or code review needed, not for functional aspects, and then the primary issue would have been revealed. You can argue about the robustness of the coding (there and back again, a decimal's tale) later but at least checkout would be passing valid data onwards.
I did. It used to all work.woowahwoo said:
Disagree. As I said, the OP could, quite easily, have tested the basket/cart at various order values and quantities, spamming the inputs with garbage etc... No selenium or code review needed, not for functional aspects, and then the primary issue would have been revealed. You can argue about the robustness of the coding (there and back again, a decimal's tale) later but at least checkout would be passing valid data onwards.
No, he (sorry for my pronouns) couldn't have, because it worked initially, it wasn't really his job, and the problem has only recently come to light. He doesn't have the time to fuzz test manually (nobody does that anyway, that's an automated test), nor the knowledge to set up a full test harness (who would apart from a programmer?), but as I've already said, no amount of testing, when it worked, would have shown the fragility of the code.droopsnoot said:
Have a look at the functions used to access the database as well - if they look like this, then you may have another issue any time soon: mysql_query("some text") or mysql_close(). Functions that use the old-style calls (as opposed to mysqli or PDO) won't work when your host upgrades to PHP7.
I can see a config.php file that is full of references like this :-$result = mysql_query($sql);
So I guess that's me f
ked Smart. Dave_ST220 said:
Haha, why does that not surprise me! This is making sense, the amounts are stored in a DB so I guess this code is trying to strip the commas??
Yup!In the absent programmers defense, if he didn't set up that database he's as much a victim as you are.
Developers very rarely have a "green field site" when they start building - normally a large chuck of any job is understanding what bodges already exist in the current systems in use and how to get their bits to work with them, for the smallest amount of time and effort.
It might be better to say "all of this is crap, rip it out and start again" but that doesn't get you the contract, and that's "out of scope". Idealists tend to starve, whereas realists get to eat.
So you take the job on and do your best... When it works, fine! When it doesn't, finger pointing isn't all black and white...
You'll probably find that the code itself is fine (apart from the mistake you found). It's the rest if the infrastructure around it that caused the issue really.
Did you have a test system, or was testing done on the live system with small transactions? If the latter that's why this bug went undiscovered until now...
The developer wrote the lot, from scratch. I gave a detailed spec of what was needed & off we went. It was tested in the sandbox for the 3DS & processor & tested again when live. I have no idea why it stopped working other than an Apache upgrade or PHP upgrade. Starting again would obviously be the best bet but this system does everything, invoicing, payments, products etc etc. It would take weeks if not months to setup all again. It took them months to write it all in the first place!
As I've said I'm not here for finger pointing, I could quite easily name the person who wrote it but that isn't going to achieve anything. They left me in the st with bits not working which over the years I've fixed. The code IS a mess, unused parts & scripts exist so it's hard to work out what is going on. As I've said I'd pay someone to go through it all and clean it up but no one wants to do that. Sooner or later I'll have to bite the bullet and get an off the shelf package & re-write the whole site.
Anyway, another band aid that was applied got it working as it should, it takes payments and passing PCI scans so at the moment I have some more time.
One pitfall to bespoke, if the person that did it "disappears" you are left with a big fking mess & headache, never again!
ETA, this was all written in 2008/2009 by memory so it's worked fine for quite a while!
As I've said I'm not here for finger pointing, I could quite easily name the person who wrote it but that isn't going to achieve anything. They left me in the s
t with bits not working which over the years I've fixed. The code IS a mess, unused parts & scripts exist so it's hard to work out what is going on. As I've said I'd pay someone to go through it all and clean it up but no one wants to do that. Sooner or later I'll have to bite the bullet and get an off the shelf package & re-write the whole site.Anyway, another band aid that was applied got it working as it should, it takes payments and passing PCI scans so at the moment I have some more time.
One pitfall to bespoke, if the person that did it "disappears" you are left with a big f
king mess & headache, never again!ETA, this was all written in 2008/2009 by memory so it's worked fine for quite a while!
Edited by Dave_ST220 on Tuesday 9th February 08:38
Dave_ST220 said:
passing PCI scans
From the looks of the code you've posted so far, there are ways the code could potentially (again, potentially, there's nowhere near enough info here for anyone to be 100% on anything!) under (or perhaps over) charge customers, without falling foul of PCI compliance - it's not a testing panacea.Gassing Station | Computers, Gadgets & Stuff | Top of Page | What's New | My Stuff