Sorting a list by country name Announcing the arrival of Valued Associate #679: Cesar Manara Planned maintenance scheduled April 17/18, 2019 at 00:00UTC (8:00pm US/Eastern)Creating a table for use in ExcelQuerying a database with PHPIs this a good structure for my website?Website for updating a divMVC JavaScript app to display athletes by countryGet rows from a table and add rows to google chartsGetting data from a server for raffle game and saving data back to the serverUse parameter to filter results in a database without directly allowing users to access databaseDynamically displaying or hiding checkboxes based on region selection country. Also select All / None includedAdding ID to body tag from variable defined in URL
How to motivate offshore teams and trust them to deliver?
Is 1 ppb equal to 1 μg/kg?
Sorting numerically
Why did the IBM 650 use bi-quinary?
Models of set theory where not every set can be linearly ordered
ListPlot join points by nearest neighbor rather than order
Why is black pepper both grey and black?
Why there are no cargo aircraft with "flying wing" design?
Right-skewed distribution with mean equals to mode?
What are the pros and cons of Aerospike nosecones?
Is there a concise way to say "all of the X, one of each"?
How do I keep my slimes from escaping their pens?
Withdrew £2800, but only £2000 shows as withdrawn on online banking; what are my obligations?
How to recreate this effect in Photoshop?
Why does Python start at index 1 when iterating an array backwards?
What happens to sewage if there is no river near by?
Check which numbers satisfy the condition [A*B*C = A! + B! + C!]
How to bypass password on Windows XP account?
How can whole tone melodies sound more interesting?
Single word antonym of "flightless"
Java 8 stream max() function argument type Comparator vs Comparable
Can inflation occur in a positive-sum game currency system such as the Stack Exchange reputation system?
Stars Make Stars
What are 'alternative tunings' of a guitar and why would you use them? Doesn't it make it more difficult to play?
Sorting a list by country name
Announcing the arrival of Valued Associate #679: Cesar Manara
Planned maintenance scheduled April 17/18, 2019 at 00:00UTC (8:00pm US/Eastern)Creating a table for use in ExcelQuerying a database with PHPIs this a good structure for my website?Website for updating a divMVC JavaScript app to display athletes by countryGet rows from a table and add rows to google chartsGetting data from a server for raffle game and saving data back to the serverUse parameter to filter results in a database without directly allowing users to access databaseDynamically displaying or hiding checkboxes based on region selection country. Also select All / None includedAdding ID to body tag from variable defined in URL
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty,.everyoneloves__bot-mid-leaderboard:empty margin-bottom:0;
$begingroup$
A couple of weeks ago a company sent me this coding challenge:
Please write a PHP web application and send it back to me as zip file:
- Which waits for a user action, like clicking buttons. According to these actions some data (see further below) should be:
- either shown nicely formatted on the screen
- or downloaded as CSV file
- You can either download the data on each request during the runtime of your PHP program or load the data from a database (in this case do
NOT provide a DB dump, but a script which automatically transfers the
data from the remote location to the DB)
- Preferably the implementation should be written in "clean code", separate concerns using pattern like MVC, be object oriented, very
good testable, best even already contain Unit tests and maybe even
follow the KISS and SOLID principles
AND
- Country list
- The data should be a list of countries with their country code
- Please download the base data from here
- Afterwards you will have to change the whole list from "Country code - Country name" to "CountryName - CountryCode" and sorts the list
by CountryName
Is my solution okay? How can it be improved?
Controller:
<?php
defined('BASEPATH') OR exit('No direct script access allowed');
class Welcome extends CI_Controller
function __construct()
parent::__construct();
$this->load->model('country_model');
public function index()
$data = array();
if (isset($_POST['run']))
$this->_save_data();
$data['list'] = $this->country_model->get_countries(252)->result();
$this->load->helper('form');
$this->load->view('welcome_message', $data);
private function _save_data()
// Get a file into an array. In this example we'll go through HTTP to get
// the HTML source of a URL.
$lines = file('http://pastebin.com/raw.php?i=943PQQ0n');
$lineNo = 0;
$startLine = 4;
$endLine = 255;
// Loop through our array, show HTML source as HTML source; and line numbers too.
foreach ($lines as $line_num => $line)
$lineNo++;
if ($lineNo >= $startLine)
$country = explode(' ', trim(htmlspecialchars($line)));
if (!isset($country[0])
if ($lineNo == $endLine)
break;
Model:
<?php
defined('BASEPATH') OR exit('No direct script access allowed');
class Country_model extends CI_Model
public $table = 'country';
public function __construct()
// Call the CI_Model constructor
parent::__construct();
$this->load->database();
public function insert_entry($country_code, $country_name)
$data['country_code'] = $country_code;
$data['country_name'] = $country_name;
$this->db->insert($this->table, $data);
function count_by_code($country_code)
$this->db->where('country_code', $country_code);
return $this->db->count_all_results($this->table);
function get_countries($limit)
$this->db->select('country_code, country_name');
$this->db->order_by("country_name", "asc");
return $this->db->get($this->table, 0, $limit);
View:
<?php
defined('BASEPATH') OR exit('No direct script access allowed');
?><!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Welcome to My Task</title>
</head>
<body>
<div id="container">
<h1>Welcome to My Task!</h1>
<div id="body">
<?php echo form_open(); ?>
<input type="submit" value="Get Country List" name="run"/>
<?php echo form_close(); ?>
<br>
<?php
if (isset($list))
foreach ($list as $value)
echo $value->country_name . ' - ' . $value->country_code . '<br>';
?>
</div>
<p class="footer">Page rendered in <strong>elapsed_time</strong> seconds. <?php echo (ENVIRONMENT === 'development') ? 'CodeIgniter Version <strong>' . CI_VERSION . '</strong>' : '' ?></p>
</div>
</body>
</html>
php sorting mvc codeigniter
edited Jan 9 '18 at 18:16
Sᴀᴍ Onᴇᴌᴀ
10.6k62168
asked May 30 '16 at 8:08
WaqlehWaqleh
1144
$endgroup$
add a comment |
$begingroup$
A couple of weeks ago a company sent me this coding challenge:
Please write a PHP web application and send it back to me as zip file:
- Which waits for a user action, like clicking buttons. According to these actions some data (see further below) should be:
- either shown nicely formatted on the screen
- or downloaded as CSV file
- You can either download the data on each request during the runtime of your PHP program or load the data from a database (in this case do
NOT provide a DB dump, but a script which automatically transfers the
data from the remote location to the DB)
- Preferably the implementation should be written in "clean code", separate concerns using pattern like MVC, be object oriented, very
good testable, best even already contain Unit tests and maybe even
follow the KISS and SOLID principles
AND
- Country list
- The data should be a list of countries with their country code
- Please download the base data from here
- Afterwards you will have to change the whole list from "Country code - Country name" to "CountryName - CountryCode" and sorts the list
by CountryName
Is my solution okay? How can it be improved?
Controller:
<?php
defined('BASEPATH') OR exit('No direct script access allowed');
class Welcome extends CI_Controller
function __construct()
parent::__construct();
$this->load->model('country_model');
public function index()
$data = array();
if (isset($_POST['run']))
$this->_save_data();
$data['list'] = $this->country_model->get_countries(252)->result();
$this->load->helper('form');
$this->load->view('welcome_message', $data);
private function _save_data()
// Get a file into an array. In this example we'll go through HTTP to get
// the HTML source of a URL.
$lines = file('http://pastebin.com/raw.php?i=943PQQ0n');
$lineNo = 0;
$startLine = 4;
$endLine = 255;
// Loop through our array, show HTML source as HTML source; and line numbers too.
foreach ($lines as $line_num => $line)
$lineNo++;
if ($lineNo >= $startLine)
$country = explode(' ', trim(htmlspecialchars($line)));
if (!isset($country[0])
if ($lineNo == $endLine)
break;
Model:
<?php
defined('BASEPATH') OR exit('No direct script access allowed');
class Country_model extends CI_Model
public $table = 'country';
public function __construct()
// Call the CI_Model constructor
parent::__construct();
$this->load->database();
public function insert_entry($country_code, $country_name)
$data['country_code'] = $country_code;
$data['country_name'] = $country_name;
$this->db->insert($this->table, $data);
function count_by_code($country_code)
$this->db->where('country_code', $country_code);
return $this->db->count_all_results($this->table);
function get_countries($limit)
$this->db->select('country_code, country_name');
$this->db->order_by("country_name", "asc");
return $this->db->get($this->table, 0, $limit);
View:
<?php
defined('BASEPATH') OR exit('No direct script access allowed');
?><!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Welcome to My Task</title>
</head>
<body>
<div id="container">
<h1>Welcome to My Task!</h1>
<div id="body">
<?php echo form_open(); ?>
<input type="submit" value="Get Country List" name="run"/>
<?php echo form_close(); ?>
<br>
<?php
if (isset($list))
foreach ($list as $value)
echo $value->country_name . ' - ' . $value->country_code . '<br>';
?>
</div>
<p class="footer">Page rendered in <strong>elapsed_time</strong> seconds. <?php echo (ENVIRONMENT === 'development') ? 'CodeIgniter Version <strong>' . CI_VERSION . '</strong>' : '' ?></p>
</div>
</body>
</html>
php sorting mvc codeigniter
edited Jan 9 '18 at 18:16
Sᴀᴍ Onᴇᴌᴀ
10.6k62168
asked May 30 '16 at 8:08
WaqlehWaqleh
1144
$endgroup$
add a comment |
$begingroup$
A couple of weeks ago a company sent me this coding challenge:
Please write a PHP web application and send it back to me as zip file:
- Which waits for a user action, like clicking buttons. According to these actions some data (see further below) should be:
- either shown nicely formatted on the screen
- or downloaded as CSV file
- You can either download the data on each request during the runtime of your PHP program or load the data from a database (in this case do
NOT provide a DB dump, but a script which automatically transfers the
data from the remote location to the DB)
- Preferably the implementation should be written in "clean code", separate concerns using pattern like MVC, be object oriented, very
good testable, best even already contain Unit tests and maybe even
follow the KISS and SOLID principles
AND
- Country list
- The data should be a list of countries with their country code
- Please download the base data from here
- Afterwards you will have to change the whole list from "Country code - Country name" to "CountryName - CountryCode" and sorts the list
by CountryName
Is my solution okay? How can it be improved?
Controller:
<?php
defined('BASEPATH') OR exit('No direct script access allowed');
class Welcome extends CI_Controller
function __construct()
parent::__construct();
$this->load->model('country_model');
public function index()
$data = array();
if (isset($_POST['run']))
$this->_save_data();
$data['list'] = $this->country_model->get_countries(252)->result();
$this->load->helper('form');
$this->load->view('welcome_message', $data);
private function _save_data()
// Get a file into an array. In this example we'll go through HTTP to get
// the HTML source of a URL.
$lines = file('http://pastebin.com/raw.php?i=943PQQ0n');
$lineNo = 0;
$startLine = 4;
$endLine = 255;
// Loop through our array, show HTML source as HTML source; and line numbers too.
foreach ($lines as $line_num => $line)
$lineNo++;
if ($lineNo >= $startLine)
$country = explode(' ', trim(htmlspecialchars($line)));
if (!isset($country[0])
if ($lineNo == $endLine)
break;
Model:
<?php
defined('BASEPATH') OR exit('No direct script access allowed');
class Country_model extends CI_Model
public $table = 'country';
public function __construct()
// Call the CI_Model constructor
parent::__construct();
$this->load->database();
public function insert_entry($country_code, $country_name)
$data['country_code'] = $country_code;
$data['country_name'] = $country_name;
$this->db->insert($this->table, $data);
function count_by_code($country_code)
$this->db->where('country_code', $country_code);
return $this->db->count_all_results($this->table);
function get_countries($limit)
$this->db->select('country_code, country_name');
$this->db->order_by("country_name", "asc");
return $this->db->get($this->table, 0, $limit);
View:
<?php
defined('BASEPATH') OR exit('No direct script access allowed');
?><!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Welcome to My Task</title>
</head>
<body>
<div id="container">
<h1>Welcome to My Task!</h1>
<div id="body">
<?php echo form_open(); ?>
<input type="submit" value="Get Country List" name="run"/>
<?php echo form_close(); ?>
<br>
<?php
if (isset($list))
foreach ($list as $value)
echo $value->country_name . ' - ' . $value->country_code . '<br>';
?>
</div>
<p class="footer">Page rendered in <strong>elapsed_time</strong> seconds. <?php echo (ENVIRONMENT === 'development') ? 'CodeIgniter Version <strong>' . CI_VERSION . '</strong>' : '' ?></p>
</div>
</body>
</html>
php sorting mvc codeigniter
edited Jan 9 '18 at 18:16
Sᴀᴍ Onᴇᴌᴀ
10.6k62168
asked May 30 '16 at 8:08
WaqlehWaqleh
1144
$endgroup$
A couple of weeks ago a company sent me this coding challenge:
Please write a PHP web application and send it back to me as zip file:
- Which waits for a user action, like clicking buttons. According to these actions some data (see further below) should be:
- either shown nicely formatted on the screen
- or downloaded as CSV file
- You can either download the data on each request during the runtime of your PHP program or load the data from a database (in this case do
NOT provide a DB dump, but a script which automatically transfers the
data from the remote location to the DB)
- Preferably the implementation should be written in "clean code", separate concerns using pattern like MVC, be object oriented, very
good testable, best even already contain Unit tests and maybe even
follow the KISS and SOLID principles
AND
- Country list
- The data should be a list of countries with their country code
- Please download the base data from here
- Afterwards you will have to change the whole list from "Country code - Country name" to "CountryName - CountryCode" and sorts the list
by CountryName
Is my solution okay? How can it be improved?
Controller:
<?php
defined('BASEPATH') OR exit('No direct script access allowed');
class Welcome extends CI_Controller
function __construct()
parent::__construct();
$this->load->model('country_model');
public function index()
$data = array();
if (isset($_POST['run']))
$this->_save_data();
$data['list'] = $this->country_model->get_countries(252)->result();
$this->load->helper('form');
$this->load->view('welcome_message', $data);
private function _save_data()
// Get a file into an array. In this example we'll go through HTTP to get
// the HTML source of a URL.
$lines = file('http://pastebin.com/raw.php?i=943PQQ0n');
$lineNo = 0;
$startLine = 4;
$endLine = 255;
// Loop through our array, show HTML source as HTML source; and line numbers too.
foreach ($lines as $line_num => $line)
$lineNo++;
if ($lineNo >= $startLine)
$country = explode(' ', trim(htmlspecialchars($line)));
if (!isset($country[0])
if ($lineNo == $endLine)
break;
Model:
<?php
defined('BASEPATH') OR exit('No direct script access allowed');
class Country_model extends CI_Model
public $table = 'country';
public function __construct()
// Call the CI_Model constructor
parent::__construct();
$this->load->database();
public function insert_entry($country_code, $country_name)
$data['country_code'] = $country_code;
$data['country_name'] = $country_name;
$this->db->insert($this->table, $data);
function count_by_code($country_code)
$this->db->where('country_code', $country_code);
return $this->db->count_all_results($this->table);
function get_countries($limit)
$this->db->select('country_code, country_name');
$this->db->order_by("country_name", "asc");
return $this->db->get($this->table, 0, $limit);
View:
<?php
defined('BASEPATH') OR exit('No direct script access allowed');
?><!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Welcome to My Task</title>
</head>
<body>
<div id="container">
<h1>Welcome to My Task!</h1>
<div id="body">
<?php echo form_open(); ?>
<input type="submit" value="Get Country List" name="run"/>
<?php echo form_close(); ?>
<br>
<?php
if (isset($list))
foreach ($list as $value)
echo $value->country_name . ' - ' . $value->country_code . '<br>';
?>
</div>
<p class="footer">Page rendered in <strong>elapsed_time</strong> seconds. <?php echo (ENVIRONMENT === 'development') ? 'CodeIgniter Version <strong>' . CI_VERSION . '</strong>' : '' ?></p>
</div>
</body>
</html>
php sorting mvc codeigniter
php sorting mvc codeigniter
edited Jan 9 '18 at 18:16
Sᴀᴍ Onᴇᴌᴀ
10.6k62168
asked May 30 '16 at 8:08
WaqlehWaqleh
1144
edited Jan 9 '18 at 18:16
Sᴀᴍ Onᴇᴌᴀ
10.6k62168
asked May 30 '16 at 8:08
WaqlehWaqleh
1144
edited Jan 9 '18 at 18:16
Sᴀᴍ Onᴇᴌᴀ
10.6k62168
edited Jan 9 '18 at 18:16
Sᴀᴍ Onᴇᴌᴀ
10.6k62168
edited Jan 9 '18 at 18:16
Sᴀᴍ Onᴇᴌᴀ
10.6k62168
10.6k62168
asked May 30 '16 at 8:08
WaqlehWaqleh
1144
asked May 30 '16 at 8:08
WaqlehWaqleh
1144
asked May 30 '16 at 8:08
WaqlehWaqleh
1144
1144
add a comment |
add a comment |
2 Answers
2
active
oldest
votes
$begingroup$
Looks pretty good. Here are just some thoughts but nothing major:
It would be more future-proof (and probably efficient) to use regular expressions in lieu of parsing hard-coded line numbers (in case the data has rows added later):
For the
_save_data()you could parse like so:$string = 'THE CONTENT FROM THE COUNTRIES LIST PAGE';
$sub = preg_replace('/.+?(?=ADsss)/s', '', $string); // this strips the pre-text
$list = preg_split('/$R?^/m', $sub); // This splits by line
$countries = array();
foreach ($list as $item)
$arr = explode(' ', $item);
$countries[$arr[0]] = $arr[1];
asort($countries); // sort by values
//print_r($countries);
/** Array
(
[AF] => Afghanistan
[AL] => Albania
[DZ] => Algeria
[AS] => American Samoa
[AD] => Andorra
... */This way you do not need to pass limit to
get_countries().You could also use a view helper or template for the output section to better format.
<br>can get pretty ugly so I would do a list (<ul>) or something. Overall, it seems pretty logical.
edited Jun 6 '16 at 2:49
Jamal♦
30.6k11121227
answered Jun 6 '16 at 1:20
TomanowTomanow
1214
$endgroup$
add a comment |
$begingroup$
Feedback
The code looks good so far. There appears to be good separation between the model, the view and the controller. I like how the model methods are concise - none more than 4 lines. The controller method _save_data() is a little on the lengthy side but hopefully the feedback below and in the answer by Tomanow will allow you to improve that method.
Suggestions
Regular expression
I agree with Tomanow's answer (except that you might not need to worry about stripping the pre-text, presuming that the pattern matching only matches country codes and names). A regular expression can be used in _save_data() to match each relevant row, and using a named sub-pattern, the code and name of each country can be selected:
$pattern = '/^'. //beginning of line
'(?P<country_code>[A-Za-z]2,4)'. // named sub-pattern for code: 2-4 alpha chars
's3'. //3 whitespace characters
'(?P<country_name>[A-Za-z()s.,'-]+)'. // named sub-pattern for name
'$/'; //end of line
If that pattern matches any rows, $matches['code'] will have the country code and $matches['name'] will have the country name. In theory, $matches could be sent to the insert_entry() model method, though that might be a weird design to accept the fields to insert directly (and the numeric indexes might need to be removed).
In theory the logic involving $startLine and $endLine can be removed by simply checking of the pattern matches.
$lines = file('http://pastebin.com/raw.php?i=943PQQ0n');
$pattern = '/^(?P<country_code>[A-Za-z]2,4)s3(?P<country_name>[A-Za-z()s.,'-]+)$/';
// Loop through our array, show HTML source as HTML source; and line numbers too.
foreach ($lines as $line_num => $line)
preg_match($pattern, $line, $matches);
if (count($matches))
$count = $this->country_model->count_by_code($matches['country_code']);
if (!$count)
$this->country_model->insert_entry($matches['country_code'], $matches['country_name']);
See a demonstration of the matching here in this playground example.
Useless variable $lineNo
In your method _save_data() there is a variable $lineNo that gets incremented each time. If you needed the count of each line, the variable $line_num (from the foreach statement) could be used instead (and added 1 to)...
foreach ($lines as $line_num => $line) {
$lineNo++;
answered Jan 9 '18 at 18:15
Sᴀᴍ OnᴇᴌᴀSᴀᴍ Onᴇᴌᴀ
10.6k62168
$endgroup$
add a comment |
Your Answer
StackExchange.ifUsing("editor", function ()
StackExchange.using("externalEditor", function ()
StackExchange.using("snippets", function ()
StackExchange.snippets.init();
);
);
, "code-snippets");
StackExchange.ready(function()
var channelOptions =
tags: "".split(" "),
id: "196"
;
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function()
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled)
StackExchange.using("snippets", function()
createEditor();
);
else
createEditor();
);
function createEditor()
StackExchange.prepareEditor(
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader:
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
,
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
);
);
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
var $window = $(window),
onScroll = function(e)
var $elem = $('.new-login-left'),
docViewTop = $window.scrollTop(),
docViewBottom = docViewTop + $window.height(),
elemTop = $elem.offset().top,
elemBottom = elemTop + $elem.height();
if ((docViewTop elemBottom))
StackExchange.using('gps', function() StackExchange.gps.track('embedded_signup_form.view', location: 'question_page' ); );
$window.unbind('scroll', onScroll);
;
$window.on('scroll', onScroll);
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f129657%2fsorting-a-list-by-country-name%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
2 Answers
2
active
oldest
votes
2 Answers
2
active
oldest
votes
active
oldest
votes
active
oldest
votes
$begingroup$
Looks pretty good. Here are just some thoughts but nothing major:
It would be more future-proof (and probably efficient) to use regular expressions in lieu of parsing hard-coded line numbers (in case the data has rows added later):
For the
_save_data()you could parse like so:$string = 'THE CONTENT FROM THE COUNTRIES LIST PAGE';
$sub = preg_replace('/.+?(?=ADsss)/s', '', $string); // this strips the pre-text
$list = preg_split('/$R?^/m', $sub); // This splits by line
$countries = array();
foreach ($list as $item)
$arr = explode(' ', $item);
$countries[$arr[0]] = $arr[1];
asort($countries); // sort by values
//print_r($countries);
/** Array
(
[AF] => Afghanistan
[AL] => Albania
[DZ] => Algeria
[AS] => American Samoa
[AD] => Andorra
... */This way you do not need to pass limit to
get_countries().You could also use a view helper or template for the output section to better format.
<br>can get pretty ugly so I would do a list (<ul>) or something. Overall, it seems pretty logical.
edited Jun 6 '16 at 2:49
Jamal♦
30.6k11121227
answered Jun 6 '16 at 1:20
TomanowTomanow
1214
$endgroup$
add a comment |
$begingroup$
Looks pretty good. Here are just some thoughts but nothing major:
It would be more future-proof (and probably efficient) to use regular expressions in lieu of parsing hard-coded line numbers (in case the data has rows added later):
For the
_save_data()you could parse like so:$string = 'THE CONTENT FROM THE COUNTRIES LIST PAGE';
$sub = preg_replace('/.+?(?=ADsss)/s', '', $string); // this strips the pre-text
$list = preg_split('/$R?^/m', $sub); // This splits by line
$countries = array();
foreach ($list as $item)
$arr = explode(' ', $item);
$countries[$arr[0]] = $arr[1];
asort($countries); // sort by values
//print_r($countries);
/** Array
(
[AF] => Afghanistan
[AL] => Albania
[DZ] => Algeria
[AS] => American Samoa
[AD] => Andorra
... */This way you do not need to pass limit to
get_countries().You could also use a view helper or template for the output section to better format.
<br>can get pretty ugly so I would do a list (<ul>) or something. Overall, it seems pretty logical.
edited Jun 6 '16 at 2:49
Jamal♦
30.6k11121227
answered Jun 6 '16 at 1:20
TomanowTomanow
1214
$endgroup$
add a comment |
$begingroup$
Looks pretty good. Here are just some thoughts but nothing major:
It would be more future-proof (and probably efficient) to use regular expressions in lieu of parsing hard-coded line numbers (in case the data has rows added later):
For the
_save_data()you could parse like so:$string = 'THE CONTENT FROM THE COUNTRIES LIST PAGE';
$sub = preg_replace('/.+?(?=ADsss)/s', '', $string); // this strips the pre-text
$list = preg_split('/$R?^/m', $sub); // This splits by line
$countries = array();
foreach ($list as $item)
$arr = explode(' ', $item);
$countries[$arr[0]] = $arr[1];
asort($countries); // sort by values
//print_r($countries);
/** Array
(
[AF] => Afghanistan
[AL] => Albania
[DZ] => Algeria
[AS] => American Samoa
[AD] => Andorra
... */This way you do not need to pass limit to
get_countries().You could also use a view helper or template for the output section to better format.
<br>can get pretty ugly so I would do a list (<ul>) or something. Overall, it seems pretty logical.
edited Jun 6 '16 at 2:49
Jamal♦
30.6k11121227
answered Jun 6 '16 at 1:20
TomanowTomanow
1214
$endgroup$
Looks pretty good. Here are just some thoughts but nothing major:
It would be more future-proof (and probably efficient) to use regular expressions in lieu of parsing hard-coded line numbers (in case the data has rows added later):
For the
_save_data()you could parse like so:$string = 'THE CONTENT FROM THE COUNTRIES LIST PAGE';
$sub = preg_replace('/.+?(?=ADsss)/s', '', $string); // this strips the pre-text
$list = preg_split('/$R?^/m', $sub); // This splits by line
$countries = array();
foreach ($list as $item)
$arr = explode(' ', $item);
$countries[$arr[0]] = $arr[1];
asort($countries); // sort by values
//print_r($countries);
/** Array
(
[AF] => Afghanistan
[AL] => Albania
[DZ] => Algeria
[AS] => American Samoa
[AD] => Andorra
... */This way you do not need to pass limit to
get_countries().You could also use a view helper or template for the output section to better format.
<br>can get pretty ugly so I would do a list (<ul>) or something. Overall, it seems pretty logical.
edited Jun 6 '16 at 2:49
Jamal♦
30.6k11121227
answered Jun 6 '16 at 1:20
TomanowTomanow
1214
edited Jun 6 '16 at 2:49
Jamal♦
30.6k11121227
edited Jun 6 '16 at 2:49
Jamal♦
30.6k11121227
edited Jun 6 '16 at 2:49
Jamal♦
30.6k11121227
30.6k11121227
answered Jun 6 '16 at 1:20
TomanowTomanow
1214
answered Jun 6 '16 at 1:20
TomanowTomanow
1214
answered Jun 6 '16 at 1:20
TomanowTomanow
1214
1214
add a comment |
add a comment |
$begingroup$
Feedback
The code looks good so far. There appears to be good separation between the model, the view and the controller. I like how the model methods are concise - none more than 4 lines. The controller method _save_data() is a little on the lengthy side but hopefully the feedback below and in the answer by Tomanow will allow you to improve that method.
Suggestions
Regular expression
I agree with Tomanow's answer (except that you might not need to worry about stripping the pre-text, presuming that the pattern matching only matches country codes and names). A regular expression can be used in _save_data() to match each relevant row, and using a named sub-pattern, the code and name of each country can be selected:
$pattern = '/^'. //beginning of line
'(?P<country_code>[A-Za-z]2,4)'. // named sub-pattern for code: 2-4 alpha chars
's3'. //3 whitespace characters
'(?P<country_name>[A-Za-z()s.,'-]+)'. // named sub-pattern for name
'$/'; //end of line
If that pattern matches any rows, $matches['code'] will have the country code and $matches['name'] will have the country name. In theory, $matches could be sent to the insert_entry() model method, though that might be a weird design to accept the fields to insert directly (and the numeric indexes might need to be removed).
In theory the logic involving $startLine and $endLine can be removed by simply checking of the pattern matches.
$lines = file('http://pastebin.com/raw.php?i=943PQQ0n');
$pattern = '/^(?P<country_code>[A-Za-z]2,4)s3(?P<country_name>[A-Za-z()s.,'-]+)$/';
// Loop through our array, show HTML source as HTML source; and line numbers too.
foreach ($lines as $line_num => $line)
preg_match($pattern, $line, $matches);
if (count($matches))
$count = $this->country_model->count_by_code($matches['country_code']);
if (!$count)
$this->country_model->insert_entry($matches['country_code'], $matches['country_name']);
See a demonstration of the matching here in this playground example.
Useless variable $lineNo
In your method _save_data() there is a variable $lineNo that gets incremented each time. If you needed the count of each line, the variable $line_num (from the foreach statement) could be used instead (and added 1 to)...
foreach ($lines as $line_num => $line) {
$lineNo++;
answered Jan 9 '18 at 18:15
Sᴀᴍ OnᴇᴌᴀSᴀᴍ Onᴇᴌᴀ
10.6k62168
$endgroup$
add a comment |
$begingroup$
Feedback
The code looks good so far. There appears to be good separation between the model, the view and the controller. I like how the model methods are concise - none more than 4 lines. The controller method _save_data() is a little on the lengthy side but hopefully the feedback below and in the answer by Tomanow will allow you to improve that method.
Suggestions
Regular expression
I agree with Tomanow's answer (except that you might not need to worry about stripping the pre-text, presuming that the pattern matching only matches country codes and names). A regular expression can be used in _save_data() to match each relevant row, and using a named sub-pattern, the code and name of each country can be selected:
$pattern = '/^'. //beginning of line
'(?P<country_code>[A-Za-z]2,4)'. // named sub-pattern for code: 2-4 alpha chars
's3'. //3 whitespace characters
'(?P<country_name>[A-Za-z()s.,'-]+)'. // named sub-pattern for name
'$/'; //end of line
If that pattern matches any rows, $matches['code'] will have the country code and $matches['name'] will have the country name. In theory, $matches could be sent to the insert_entry() model method, though that might be a weird design to accept the fields to insert directly (and the numeric indexes might need to be removed).
In theory the logic involving $startLine and $endLine can be removed by simply checking of the pattern matches.
$lines = file('http://pastebin.com/raw.php?i=943PQQ0n');
$pattern = '/^(?P<country_code>[A-Za-z]2,4)s3(?P<country_name>[A-Za-z()s.,'-]+)$/';
// Loop through our array, show HTML source as HTML source; and line numbers too.
foreach ($lines as $line_num => $line)
preg_match($pattern, $line, $matches);
if (count($matches))
$count = $this->country_model->count_by_code($matches['country_code']);
if (!$count)
$this->country_model->insert_entry($matches['country_code'], $matches['country_name']);
See a demonstration of the matching here in this playground example.
Useless variable $lineNo
In your method _save_data() there is a variable $lineNo that gets incremented each time. If you needed the count of each line, the variable $line_num (from the foreach statement) could be used instead (and added 1 to)...
foreach ($lines as $line_num => $line) {
$lineNo++;
answered Jan 9 '18 at 18:15
Sᴀᴍ OnᴇᴌᴀSᴀᴍ Onᴇᴌᴀ
10.6k62168
$endgroup$
add a comment |
$begingroup$
Feedback
The code looks good so far. There appears to be good separation between the model, the view and the controller. I like how the model methods are concise - none more than 4 lines. The controller method _save_data() is a little on the lengthy side but hopefully the feedback below and in the answer by Tomanow will allow you to improve that method.
Suggestions
Regular expression
I agree with Tomanow's answer (except that you might not need to worry about stripping the pre-text, presuming that the pattern matching only matches country codes and names). A regular expression can be used in _save_data() to match each relevant row, and using a named sub-pattern, the code and name of each country can be selected:
$pattern = '/^'. //beginning of line
'(?P<country_code>[A-Za-z]2,4)'. // named sub-pattern for code: 2-4 alpha chars
's3'. //3 whitespace characters
'(?P<country_name>[A-Za-z()s.,'-]+)'. // named sub-pattern for name
'$/'; //end of line
If that pattern matches any rows, $matches['code'] will have the country code and $matches['name'] will have the country name. In theory, $matches could be sent to the insert_entry() model method, though that might be a weird design to accept the fields to insert directly (and the numeric indexes might need to be removed).
In theory the logic involving $startLine and $endLine can be removed by simply checking of the pattern matches.
$lines = file('http://pastebin.com/raw.php?i=943PQQ0n');
$pattern = '/^(?P<country_code>[A-Za-z]2,4)s3(?P<country_name>[A-Za-z()s.,'-]+)$/';
// Loop through our array, show HTML source as HTML source; and line numbers too.
foreach ($lines as $line_num => $line)
preg_match($pattern, $line, $matches);
if (count($matches))
$count = $this->country_model->count_by_code($matches['country_code']);
if (!$count)
$this->country_model->insert_entry($matches['country_code'], $matches['country_name']);
See a demonstration of the matching here in this playground example.
Useless variable $lineNo
In your method _save_data() there is a variable $lineNo that gets incremented each time. If you needed the count of each line, the variable $line_num (from the foreach statement) could be used instead (and added 1 to)...
foreach ($lines as $line_num => $line) {
$lineNo++;
answered Jan 9 '18 at 18:15
Sᴀᴍ OnᴇᴌᴀSᴀᴍ Onᴇᴌᴀ
10.6k62168
$endgroup$
Feedback
The code looks good so far. There appears to be good separation between the model, the view and the controller. I like how the model methods are concise - none more than 4 lines. The controller method _save_data() is a little on the lengthy side but hopefully the feedback below and in the answer by Tomanow will allow you to improve that method.
Suggestions
Regular expression
I agree with Tomanow's answer (except that you might not need to worry about stripping the pre-text, presuming that the pattern matching only matches country codes and names). A regular expression can be used in _save_data() to match each relevant row, and using a named sub-pattern, the code and name of each country can be selected:
$pattern = '/^'. //beginning of line
'(?P<country_code>[A-Za-z]2,4)'. // named sub-pattern for code: 2-4 alpha chars
's3'. //3 whitespace characters
'(?P<country_name>[A-Za-z()s.,'-]+)'. // named sub-pattern for name
'$/'; //end of line
If that pattern matches any rows, $matches['code'] will have the country code and $matches['name'] will have the country name. In theory, $matches could be sent to the insert_entry() model method, though that might be a weird design to accept the fields to insert directly (and the numeric indexes might need to be removed).
In theory the logic involving $startLine and $endLine can be removed by simply checking of the pattern matches.
$lines = file('http://pastebin.com/raw.php?i=943PQQ0n');
$pattern = '/^(?P<country_code>[A-Za-z]2,4)s3(?P<country_name>[A-Za-z()s.,'-]+)$/';
// Loop through our array, show HTML source as HTML source; and line numbers too.
foreach ($lines as $line_num => $line)
preg_match($pattern, $line, $matches);
if (count($matches))
$count = $this->country_model->count_by_code($matches['country_code']);
if (!$count)
$this->country_model->insert_entry($matches['country_code'], $matches['country_name']);
See a demonstration of the matching here in this playground example.
Useless variable $lineNo
In your method _save_data() there is a variable $lineNo that gets incremented each time. If you needed the count of each line, the variable $line_num (from the foreach statement) could be used instead (and added 1 to)...
foreach ($lines as $line_num => $line) {
$lineNo++;
answered Jan 9 '18 at 18:15
Sᴀᴍ OnᴇᴌᴀSᴀᴍ Onᴇᴌᴀ
10.6k62168
edited 5 mins ago
answered Jan 9 '18 at 18:15
Sᴀᴍ OnᴇᴌᴀSᴀᴍ Onᴇᴌᴀ
10.6k62168
answered Jan 9 '18 at 18:15
Sᴀᴍ OnᴇᴌᴀSᴀᴍ Onᴇᴌᴀ
10.6k62168
answered Jan 9 '18 at 18:15
Sᴀᴍ OnᴇᴌᴀSᴀᴍ Onᴇᴌᴀ
10.6k62168
10.6k62168
add a comment |
add a comment |
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
var $window = $(window),
onScroll = function(e)
var $elem = $('.new-login-left'),
docViewTop = $window.scrollTop(),
docViewBottom = docViewTop + $window.height(),
elemTop = $elem.offset().top,
elemBottom = elemTop + $elem.height();
if ((docViewTop elemBottom))
StackExchange.using('gps', function() StackExchange.gps.track('embedded_signup_form.view', location: 'question_page' ); );
$window.unbind('scroll', onScroll);
;
$window.on('scroll', onScroll);
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f129657%2fsorting-a-list-by-country-name%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
var $window = $(window),
onScroll = function(e)
var $elem = $('.new-login-left'),
docViewTop = $window.scrollTop(),
docViewBottom = docViewTop + $window.height(),
elemTop = $elem.offset().top,
elemBottom = elemTop + $elem.height();
if ((docViewTop elemBottom))
StackExchange.using('gps', function() StackExchange.gps.track('embedded_signup_form.view', location: 'question_page' ); );
$window.unbind('scroll', onScroll);
;
$window.on('scroll', onScroll);
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
var $window = $(window),
onScroll = function(e)
var $elem = $('.new-login-left'),
docViewTop = $window.scrollTop(),
docViewBottom = docViewTop + $window.height(),
elemTop = $elem.offset().top,
elemBottom = elemTop + $elem.height();
if ((docViewTop elemBottom))
StackExchange.using('gps', function() StackExchange.gps.track('embedded_signup_form.view', location: 'question_page' ); );
$window.unbind('scroll', onScroll);
;
$window.on('scroll', onScroll);
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
var $window = $(window),
onScroll = function(e)
var $elem = $('.new-login-left'),
docViewTop = $window.scrollTop(),
docViewBottom = docViewTop + $window.height(),
elemTop = $elem.offset().top,
elemBottom = elemTop + $elem.height();
if ((docViewTop elemBottom))
StackExchange.using('gps', function() StackExchange.gps.track('embedded_signup_form.view', location: 'question_page' ); );
$window.unbind('scroll', onScroll);
;
$window.on('scroll', onScroll);
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
