Есть такая задача: Из чисел от 1го до 1000 выбрать все числа которые делятся только на единицу и на себя.
Задача была задана на собеседовании и мною был написан этот код. Первый цикл создает массив, второй перебирает все числа, третий перебирает все числа ниже возвращенного во 2м цикле и выполняет операцию по поиску целого числа от деления. Если нашлось целое число то поднимается флаг и цисло не выводится на экран т.к. не подходит.
<?php
$arrPrime = array();
for ($i=0;$i<1000;$i++)
{
$arrPrime[$i]=$i+1;
}
foreach($arrPrime as $item)
{
$flag=true;
for ($i=2;$i<$item;$i++)
{
$varible = $item%$i;
if( $varible==0)
{
$flag=false;
}
}
if($flag)
{
echo $item.'<br>';
}
}
?>
В постановке задачи не стояла задача оптимизации, я специально спросил про алгоритм перебора, цель была написать, в итоге:
Чуваку проводившему интервью не понравился мой стиль написания кода. У меня 3 года в программировании на PHP, вроде пишу нормальный код.
Подскажите что не так в стиле кода написанного мной, спасибо!
Спустя 9 минут, 32 секунды (5.08.2011 - 00:39) Winston написал(а):
-Ну первое зло это - табуляторы.
-Лично мне не нравится, что нету пробелов, между операторами например > < или =
-Если тело какого-то оператора состоит всего из одной строки то фигурные скобки не нужны.
А так вроде все нормально.
Если интересно можешь это почитать.
-Лично мне не нравится, что нету пробелов, между операторами например > < или =
-Если тело какого-то оператора состоит всего из одной строки то фигурные скобки не нужны.
А так вроде все нормально.
Вот так красивее
<?php
$arrPrime = array();
for($i = 0; $i < 1000; $i++)
$arrPrime[$i] = $i + 1;
foreach($arrPrime as $item)
{
$flag = true;
for($i = 2; $i < $item; $i++)
{
$varible = $item % $i;
if($varible == 0)
$flag = false;
}
if($flag)
echo $item . '<br>';
}
?>
Если интересно можешь это почитать.
Спустя 6 минут, 50 секунд (5.08.2011 - 00:46) razgon написал(а):
Спасибо!!! К сожалению не могу плюсануть
Спустя 1 час, 15 минут, 16 секунд (5.08.2011 - 02:01) waldicom написал(а):
Вот стандарты от Zend: http://framework.zend.com/manual/en/coding-standard.html
Спустя 20 минут, 20 секунд (5.08.2011 - 02:21) Nikitian написал(а):
Вопрос оптимизации должен стоять всегда. Если вы пишете неоптимальный код только потому, что вам не сказали обратное, значит либо всегда надо говорить: "Пиши оптимальный код", либо отказать. Написание неоптимального кода для заказчика - лишние затраты на железо.
Как минимум на 1 цикл можно упростить код: создание массива не имеет смысла: достаточно просто сделать цикл до нужного числа и в нём вторым циклом проверять на простоту числа.
Плюс цикл проверки не прерывается при нахождении делителя, т.е. всегда будет проходить до конца не имея смысла.
Ещё можно ограничить цикл проверки на делимость до половины проверяемого числа и сразу проверять на чётность.
Вот именно поэтому я полностью согласен с рекрутёром.
Как минимум на 1 цикл можно упростить код: создание массива не имеет смысла: достаточно просто сделать цикл до нужного числа и в нём вторым циклом проверять на простоту числа.
Плюс цикл проверки не прерывается при нахождении делителя, т.е. всегда будет проходить до конца не имея смысла.
Ещё можно ограничить цикл проверки на делимость до половины проверяемого числа и сразу проверять на чётность.
Вот именно поэтому я полностью согласен с рекрутёром.
Спустя 2 часа, 8 минут, 1 секунда (5.08.2011 - 04:30) razgon написал(а):
Большое спасибо! Буду иметь ввиду.
Спустя 2 часа, 51 минута, 59 секунд (5.08.2011 - 07:21) razgon написал(а):
<?php
echo 2;
$j=3;
$f=1;
while($j!=999)
{
$flag=true;
for ($i=2; $i<$j; $i++)
{
if( $j%$i==0)
{
$flag=false;
break;
}
if($i > 30) break;
}
if($flag)
{
echo $j.'<br>';
$f++;
}
$j++; $j++;
}
?>
Скорость увеличилась в 100+ раз.
Так?
Спустя 9 минут, 17 секунд (5.08.2011 - 07:31) Michael написал(а):
Конечно код не понравился(это к первому коду). Алгоритм его плохо обдуман, неосновательно так сказать.
- когда устанавливаешь flag=false нужно break из цикла сделать.
- проверяешь ты например число 800. Зачем его проверять на число 401, 402, ...799 - ясно же что он 800 не поделится на числа большие чем его половина. Также и треть ...
А вообще тут посмотри
- когда устанавливаешь flag=false нужно break из цикла сделать.
- проверяешь ты например число 800. Зачем его проверять на число 401, 402, ...799 - ясно же что он 800 не поделится на числа большие чем его половина. Также и треть ...
А вообще тут посмотри
Спустя 3 минуты, 56 секунд (5.08.2011 - 07:35) Michael написал(а):
Второй код не так очевиден.
$j += 2; // не?
По стилю надо отделять операторы от операндов. не так $j=3;, а так $j = 3;
if($i > 30) break;- назначение?
$j++; $j++;
$j += 2; // не?
По стилю надо отделять операторы от операндов. не так $j=3;, а так $j = 3;
Спустя 6 минут, 16 секунд (5.08.2011 - 07:41) redreem написал(а):
если уж вам так принципиально, то посмотрите тут
Спустя 5 часов, 43 минуты, 42 секунды (5.08.2011 - 13:25) Nikitian написал(а):
Накидал небольшой код. Вроде ничего лишнего
На моём сервере больше 75к значений (последнее 74959) за 30 секунд не считает
<?php
if(array_key_exists('digit',$_GET) && $_GET['digit']>=1){
$i=1;
$start = microtime(true);
while($i<=$_GET['digit']){
$j=2;
$end=ceil($i/2);
while($j<=$end){
if($i%$j==0){
break;
}
$j++;
}
if($j>$end){
echo$i.'<br />';
}
$i++;
}
echo '<p>'.(microtime(true)-$start).' sec</p>';
}
?>
<form action="" method="get">
<input type="text" name="digit" /><input type="submit" />
</form>
На моём сервере больше 75к значений (последнее 74959) за 30 секунд не считает