Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dup code in catalog/controller/checkout/manual.php #1746

Closed
benej60 opened this issue Aug 15, 2014 · 0 comments
Closed

dup code in catalog/controller/checkout/manual.php #1746

benej60 opened this issue Aug 15, 2014 · 0 comments

Comments

@benej60
Copy link

benej60 commented Aug 15, 2014

Not a bug, per se, just some wasted cycles.

Looking at the latest 1.5.6.* branch, there's something fishy in catalog/controller/checkout/manual.php around line 282. Unless I'm missing something, it looks like this code is repeated twice at lines 282 and 298:

$this->load->model('localisation/country');

$country_info = $this->model_localisation_country->getCountry($this->request->post['shipping_country_id']);

if ($country_info && $country_info['postcode_required'] && (utf8_strlen($this->request->post['shipping_postcode']) < 2) || (utf8_strlen($this->request->post['shipping_postcode']) > 10)) {
    $json['error']['shipping']['postcode'] = $this->language->get('error_postcode');
}

Not to mention that "$this->load->model('localisation/country');" is already called a few lines before. I'm not sure the dup code actually causes any issues, it looks like the code is just executed twice without causing any problem. I'm assuming the first instance of the above code should be deleted so the POST params can be checked before the second instance of the code, but it's also odd that even if the required params are missing (e.g. shipping_country_id) it sets the error message but still goes ahead and calls getCountry() anyway, though maybe it handles that cleanly, I haven't tested it.

It may also be that you don't care too much about this file... I didn't see it in the 2.0 master branch, at least not in the same folder.

Ben

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants