Помогите оценить код
От: Аноним  
Дата: 27.08.07 12:33
Оценка: :)))
Уважаемые форумчане!

Помогите пожалуйста объективно оценить код, и квалификацию программиста который его написал. Было дано небольшое стандртное тестовое задание — написать функцию которая переворачивает строку и фунцию подсчёта поднятых битов в байте. Работа проводилась на компьютере, в VC6 примерно 15-20 минут он возился.

#include <iostream.h>
#include <string.h>

typedef unsigned int UINT;
enum ErrStat
{
    OK = 0,
    SomeStringError = 1000,
    NotANSIString,
};

//
// Функция пеерворачивает символы в строке меняя их местами
//
ErrStat StrRevTest(char * str)
{
    char temp = '\0';
    UINT StrLen = 0;
    try
    {
        StrLen = strlen(str);
    }
    catch (...){return NotANSIString;}
    UINT StrLenMinOne = StrLen-1;
    for (UINT i=0; i<StrLen/2; i++)
    {
        temp = str[i];
        str[i] = str[StrLenMinOne-i];
        str[StrLenMinOne-i] = temp;
    }
    return OK;
}

#define BYTE_BIT_SIZE 8

template <typename T>
UINT CalcNumOfBits(T Data)
{
    UINT Result=0;
    for (UINT i=0; i<BYTE_BIT_SIZE*sizeof(T); i++)
    {
        Result += Data & 0x1;
        Data = Data>>1;
    }
    return Result;
}

int main (void)
{
    char str1[] = "abcdef";
    char str2[] = "abcdefg";
    StrRevTest(str1);
    StrRevTest(str2);
    cout<<str1<<endl<<str2<<endl;
    int num = CalcNumOfBits(0);
    num = CalcNumOfBits(1);
    num = CalcNumOfBits(2);
    num = CalcNumOfBits(3);
    num = CalcNumOfBits(4);
    num = CalcNumOfBits(5);
    int pause;
    cin>>pause;
    return 0;
}


Сам не силён, поэтому пишу сюда за объективной оценкой, что можно сказаьт о программисте который это написал?
Спасибо за внимание.
Re: Помогите оценить код
От: MasterZiv СССР  
Дата: 27.08.07 13:05
Оценка: 1 (1) +6
Аноним 876 пишет:
> Сам не силён, поэтому пишу сюда за объективной оценкой, что можно
> сказаьт о программисте который это написал?

Зачем давать задания, результат которых вы не в состоянии проверить ?
Posted via RSDN NNTP Server 2.1 beta
Re[2]: Помогите оценить код
От: Аноним  
Дата: 27.08.07 13:18
Оценка:
MZ>Зачем давать задания, результат которых вы не в состоянии проверить ?

Отдел только создаётся. Своих программеров что бы проводить собеседование ещё нет, а людей набирать надо. Тестовые задания мы в интернете нашли, а вот с оценкой конечно тяжело Поэтому и пишу сюда.
Re: Помогите оценить код
От: Аноним  
Дата: 27.08.07 13:21
Оценка:
Добавлю свои 5 копеек в духе анонима

A>Работа проводилась на компьютере, в VC6...

А за что-нибудь более адекватное нельзя было посадить, хотя бы тот же VC7-8
    try
    {
        StrLen = strlen(str);
    }
    catch (...){return NotANSIString;}

Странное исключение. Если мы передали непонятно что, то кто сказал что оно выкинет (большинство ошибок тупо пропустит). А вот если человек позиционируется как C++ программист (а вывод то потоковый), то строки разумно хранить в std::string, тогда и проверок таких делать не надо, если конечно же иное не было оговорено заданием
#define BYTE_BIT_SIZE 8

Излишнее. Конечно показать что ты заботишься о кроссплатформнности хорошо, но ибо:
template <typename T>
UINT CalcNumOfBits(T Data)
{
    UINT Result=0;
    for (UINT i=0; i<BYTE_BIT_SIZE*sizeof(T); i++)
    {
        Result += Data & 0x1;
        Data = Data>>1;
    }
    return Result;
}

Это конечно замахиывется на работу со сложными типами, да вот
Data & 0x1;
всё выдаёт, уж лучше
while(data)
{
Result += Data & 0x1;
Data = Data>>1;}
}

А вообще если надо было посчитать именно в байте, то для этого есть
unsigned NumOfBitts(char data)
{
   int cnt = 0;
   while(data)
   {
    cnt += data%2;
    data /= 2;
   }
   return cnt;
}

IMHO человек не должен показывать что он знает исключения и шаблоны, а всего лишь наиболее точно выполнять поставленные задачи.

Да, ещё я против любителей UINT и DWORD, но это уже личное
Re: Помогите оценить код
От: OdesitVadim Украина  
Дата: 27.08.07 13:32
Оценка:
Здравствуйте, <Аноним>, Вы писали:
функция strlen имеет тип size_t. Человек объявил свой тип. стандарт гарантирует, что size_t сможет вместить размер строки, а вот unsigned int. Вполне может быть, что на некоторых платформах эти типы не совпадут.
С другой стороны зачем было вообще в данном случае обявлять свой тип да ещё с именем UINT — во вмогих компиляторах такой тип уже есть...
Потом глюки будете искать
... << RSDN@Home 1.2.0 alpha rev. 727>>
Re[2]: Помогите оценить код
От: -MyXa- Россия  
Дата: 27.08.07 13:32
Оценка:
Здравствуйте, Аноним, Вы писали:

[]

А>
А>unsigned NumOfBitts(char data)
А>{
А>   int cnt = 0;
А>   while(data)
А>   {
А>    cnt += data%2;
А>    data /= 2;
А>   }
А>   return cnt;
А>}
А>


а если это проверить так:

NumOfBitts(-1)
Если не поможет, будем действовать током... 600 Вольт (C)
Re: Помогите оценить код
От: MasterZiv СССР  
Дата: 27.08.07 13:36
Оценка:
Аноним 876 пишет:

> Помогите пожалуйста объективно оценить код, и квалификацию программиста

> который его написал. Было дано небольшое стандртное тестовое задание —

Ладно, раз уж вам так нужно....

> #include <iostream.h>

> #include <string.h>

Эти заголовки давно уже не так
называются. Ну если второй еще можно (корректно)
оставлять как <string.h>, то первый по стандарту
<iostream>

Т.е. это чел либо студент (работал на старых компиляторах),
либо просто работал на старых компиляторах.
Хотя работать оно будет.
Если неправ — поправьте.

> enum ErrStat

> {
> OK = 0,
> SomeStringError = 1000,
> NotANSIString,
> };


> ErrStat StrRevTest(char * str)

> {
> char temp = '\0';
> UINT StrLen = 0;
> try
> {
> StrLen = strlen(str);
> }
> catch (...){return NotANSIString;}

Бред полный перехватывать тут исключения — их
не будет.

> UINT StrLenMinOne = StrLen-1;

> for (UINT i=0; i<StrLen/2; i++)
> {
> temp = str[i];
> str[i] = str[StrLenMinOne-i];
> str[StrLenMinOne-i] = temp;
> }
> return OK;
> }

Но алгоритмы-то он нормально написал. По-моему это главное.
Posted via RSDN NNTP Server 2.1 beta
Re: Помогите оценить код
От: Кодт Россия  
Дата: 27.08.07 13:37
Оценка: 3 (3) +1
Здравствуйте, <Аноним>, Вы писали:

Эх, понеслась!
// Использовал legacy iostream не из STL. Наверно, до того много писал на TurboC++ / BorlandC++ <=5
А>#include <iostream.h>
А>#include <string.h>

// Если уж так хочет использовать _правильные_ типы, то для StrRevTest должен был брать size_t
// А для наколенных решений можно было и int взять.
// Разница вылезет только на huge моделях памяти, где int < ptrdiff_t (x86 huge, ia64 и т.п.)
// Ну так по-любому unsigned int для длины строки там неадекватен.
// Итого: перебдел на ровном месте
А>typedef unsigned int UINT;

// Забавный набор кодов ошибки
А>enum ErrStat
А>{
А>    OK = 0,
А>    SomeStringError = 1000, // почему-то начинающийся с 1000, и кстати, не использующийся
А>    NotANSIString,
А>};

А>//
А>// Функция пеерворачивает символы в строке меняя их местами
А>//
А>ErrStat StrRevTest(char * str)
А>{
А>    char temp = '\0';
А>    UINT StrLen = 0;

// Ловля ошибки защиты памяти плюсовым исключением - ну-ну.
// Проще было включить в контракт этой функции условие о валидности строки
// и умыть руки от юзерских глупостей
// (а вдруг на данной платформе вообще нет защиты памяти?)
А>    try
А>    {
А>        StrLen = strlen(str); // здесь ловится только ошибка чтения
А>    }
А>    catch (...){return NotANSIString;}
А>    UINT StrLenMinOne = StrLen-1; // единичку вычесть - это мы кэшируем,
А>    for (UINT i=0; i<StrLen/2; i++) // а на два делить - это уже нет
А>    {
А>        temp = str[i];
А>        str[i] = str[StrLenMinOne-i]; // а здесь не ловится ошибка записи (строка-константа?)
А>        str[StrLenMinOne-i] = temp;
А>    }
А>    return OK;
А>}
// Выводы:
// 1) недоперебдел с защитой памяти
// 2) заложился на волю компилятора и рантайма (трансляцию signal/SEH на исключения)
// 3) недоперебдел с оптимизацией (привет Дейкстре, кстати - premature и далее по тексту)

// Не знает о стандартной константе CHAR_BIT
// Кстати говоря, её явное использование может помочь лишь для раскрутки цикла
// а можно было написать код, не зависящий от неё
А>#define BYTE_BIT_SIZE 8

А>template <typename T>
А>UINT CalcNumOfBits(T Data)
А>{
А>    UINT Result=0;
// Для популярных платформ - целочисленные типы занимают всю доступную разрядную сетку.
// Но для мифической БЭСМ-6 это уже не так. Можно случайно посчитать биты в мусоре.
// Корректнее было использовать numeric_limits<T>, а кроме того, тем самым мы бы отфильтровали
// использование этой функции на нечисловых типах
А>    for (UINT i=0; i<BYTE_BIT_SIZE*sizeof(T); i++)
А>    {
А>        Result += Data & 0x1;
А>        Data = Data>>1; // как насчёт оператора >>=
// Да, кстати! он осознаёт, как эта штука будет вести себя на знаковых типах?
// На дополнительном коде, в заполненной разрядной сетке, с априорным количеством итераций
// здесь всё работает правильно.
// Убери любое из упомянутых условий - поломается.
А>    }
А>    return Result;
А>}

А>int main (void) // привет голому Си! (void)
А>{
// По поводу AV на строках: предложи заменить char str1[] на char* str1 ;)
А>    char str1[] = "abcdef";
А>    char str2[] = "abcdefg";
А>    StrRevTest(str1);
А>    StrRevTest(str2);
А>    cout<<str1<<endl<<str2<<endl;

// Уж извольте: или UINT, или int. Иначе зачем было городить огород?
А>    int num = CalcNumOfBits(0);
А>    num = CalcNumOfBits(1);
А>    num = CalcNumOfBits(2);
А>    num = CalcNumOfBits(3);
А>    num = CalcNumOfBits(4);
А>    num = CalcNumOfBits(5);
// посчитать посчитали, а вывести забыли :)
А>    int pause;
    cin>>>pause;
А>    return 0;
А>}


Резюме: товарищ не имеет практики программирования.
Он излишне сосредоточился на ненужных вещах, и к тому же сделал их неграмотно. В то же время, упустил вещи нужные.

Под опытным руководством — наверное, научится.
Хотя всё упирается в вопрос: насколько крепко он будет держаться за свой опыт программирования для бормана.
... << RSDN@Home 1.2.0 alpha rev. 655>>
Перекуём баги на фичи!
Re[3]: Помогите оценить код
От: Аноним  
Дата: 27.08.07 13:37
Оценка:
Здравствуйте, Аноним, Вы писали:

MZ>>Зачем давать задания, результат которых вы не в состоянии проверить ?

А>Отдел только создаётся. Своих программеров что бы проводить собеседование ещё нет, а людей набирать надо. Тестовые задания мы в интернете нашли, а вот с оценкой конечно тяжело Поэтому и пишу сюда.
Так создавайте в отделе о работе новую вакансию "Нужен человек для проверки тестовых заданий"
Re: Помогите оценить код
От: alzt  
Дата: 27.08.07 13:53
Оценка: 4 (2)
Здравствуйте, Аноним, Вы писали:

Озвучте зарплату, на которую собеседовается человек, а то получится, что тут кандидата залажают, а окажется, что для рассматриваемой зарплаты он просто отлично пишет и другого такого вы не найдёте.
Re[2]: Помогите оценить код
От: Аноним  
Дата: 27.08.07 14:10
Оценка:
A>Озвучте зарплату, на которую собеседовается человек, а то получится, что тут кандидата залажают, а окажется, что для рассматриваемой зарплаты он просто отлично пишет и другого такого вы не найдёте.

Планировалась вилка 9-15 тыс, в нашей глубинке эта зп так скажем выше среднего (не для программистов, а вообще по городу)
Re[2]: Помогите оценить код
От: Phoenics Россия https://sourceforge.net/projects/phengine
Дата: 27.08.07 14:28
Оценка:
A>Озвучте зарплату, на которую собеседовается человек, а то получится, что тут кандидата залажают, а окажется, что для рассматриваемой зарплаты он просто отлично пишет и другого такого вы не найдёте.

Кстати да, вопрос соотношения цена/качество как-то был упущен из виду
---=== С наилучшими пожеланиями, Phoenics ===---
_
Re[4]: Помогите оценить код
От: Кодт Россия  
Дата: 27.08.07 14:32
Оценка:
Здравствуйте, <Аноним>, Вы писали:

А>Так создавайте в отделе о работе новую вакансию "Нужен человек для проверки тестовых заданий"


А его кто будет тестировать? RSDN DDos Team?
... << RSDN@Home 1.2.0 alpha rev. 655>>
Перекуём баги на фичи!
Re: Помогите оценить код
От: SuhanovSergey  
Дата: 27.08.07 14:39
Оценка:
Здравствуйте, Аноним, Вы писали:

А>Сам не силён, поэтому пишу сюда за объективной оценкой, что можно сказаьт о программисте который это написал?

А>Спасибо за внимание.
1. У тестируемого наблюдается бардак в такой важной области C++ как обработка ошибок. Он намешал обработку исключений с кодами возврата. Причём и то и другое применил, во-первых, не к месту, во-вторых с недочётами. Не к месту, так как в данном случае вообще не требуется обработка ошибок. Недочёты: объявлен какой-то непонятный нигде не используемый код SomeStringError, использована конструкция catch (...), которая свидетельствует о неправильном поминании идеологии исключений.
Возможно, тестируемый хотел как-то показать, что он слышал об обработке ошибок, и даже знает о существовании двух стратегий. Но пытаясь "выпендриться", он сделал некачественный код, вместо того, чтобы просто выполнить задание. Это плохо, так как в новой команде он тоже возможно начнёт "выпендриваться" в ущерб проекта.
2. Видно, что программист видел только одну реализацию C++ (msvc). Хотя и пытается писать переносимый/обобщённый код, но это у него плохо получается (см. пост Аноним 7). Само по себе, отсутсвие опыта работы с другими реализациями C++, не так уж страшно, особенно, если нет требований к переносимости. Но это говорит об определянной ограниченности кругозора, о небольшом опыте работы с библиотеками C++. У толкового человека, осознанно работавшего с коммерческими библиотеками, наверняка бы сложилось понимание, когда нужно объявлять собственный UINT и BYTE_BIT_SIZE, а когда — нет.
3. Тестируемый неясно выражает свои мысли:
// Функция пеерворачивает символы в строке меняя их местами
Если бы я не прочитал начало поста, я бы не понял этого комментария.

Диагноз: junior developer.
Re[2]: Помогите оценить код
От: Socket Ниоткуда http://www.samborsky.com
Дата: 28.08.07 05:55
Оценка:
>дано небольшое стандртное тестовое задание — написать функцию которая переворачивает строку
можно сделать одной строкой зная STL
std::string name = "Hello world!!!";

reverse(name.begin(),name.end());
http://www.samborsky.com — мой блог
Re[3]: Помогите оценить код
От: Roman Odaisky Украина  
Дата: 28.08.07 08:01
Оценка: 4 (2)
Здравствуйте, Socket, Вы писали:

>>дано небольшое стандртное тестовое задание — написать функцию которая переворачивает строку

S>можно сделать одной строкой зная STL
S>std::string name = "Hello world!!!";

S>reverse(name.begin(),name.end());


Это уже обсуждалось. За reverse двойка тебе — первым делом нужно раскрыть тему std::reverse_iterator (std::string::rbegin, std::string::rend), потом, когда скажут «а разверни-ка эту строку на месте», упомянуть std::reverse, а потом уже, после «+1, но покажи, пожалуйста, свою реализацию», написать что-то вроде
template <class BidirectionalIterator>
void bicycleReverse(BidirectionalIterator first, BidirectionalIterator last)
{
    return implBicycleReverse(first, last, std::iterator_traits<Iter>::iterator_category());
}

template <class RandomAccessIterator>
void implBicycleReverse(RandomAccessIterator first, RandomAccessIterator last, std::random_access_iterator_tag)
{
    while(first < last)
    {
        std::iter_swap(first++, --last);
    }
}

template <class BidirectionalIterator>
void implBicycleReverse(BidirectionalIterator first, BidirectionalIterator last, std::bidirectional_iterator_tag)
{
    while(first != last && first != --last)
    {
        std::iter_swap(first++, last);
    }
}
До последнего не верил в пирамиду Лебедева.
Re[3]: Помогите оценить код
От: alzt  
Дата: 28.08.07 08:27
Оценка:
Здравствуйте, Socket, Вы писали:

>>дано небольшое стандртное тестовое задание — написать функцию которая переворачивает строку

S>можно сделать одной строкой зная STL
S>std::string name = "Hello world!!!";

S>reverse(name.begin(),name.end());


В данном случае stl имеет смысл использовать, если можно сразу показать проверяющему и он скажет — достаточно этого или свою реализацию написать.
Здесь совсем другая ситуация.
Хотя наверное имеет смысл писать свою реализацию, а где-нибудь внизу указать, что "я ещё и так могу".
Re[3]: Помогите оценить код
От: alzt  
Дата: 28.08.07 08:30
Оценка:
Здравствуйте, Аноним, Вы писали:

A>>Озвучте зарплату, на которую собеседовается человек, а то получится, что тут кандидата залажают, а окажется, что для рассматриваемой зарплаты он просто отлично пишет и другого такого вы не найдёте.


А>Планировалась вилка 9-15 тыс, в нашей глубинке эта зп так скажем выше среднего (не для программистов, а вообще по городу)


Скорее всего он у вас будет нормально работать и справляться со своими задачами. Какого-то супер-кода и чёткой структуры программ ждать не приходится.
Re[4]: Помогите оценить код
От: Аноним  
Дата: 28.08.07 09:10
Оценка:
Спасибо.
Re[2]: Помогите оценить код
От: Аноним  
Дата: 28.08.07 09:32
Оценка:
Здравствуйте, MasterZiv, Вы писали:

>> catch (...){return NotANSIString;}

MZ>Бред полный перехватывать тут исключения — их
MZ>не будет.
В 7-ой студии это сехи ловит
Подождите ...
Wait...
Пока на собственное сообщение не было ответов, его можно удалить.