Panel użytkownika
Nazwa użytkownika:
Hasło:
Nie masz jeszcze konta?

Losowanie bez powtórzeń - poprawność kodu

Ostatnio zmodyfikowano 2017-05-05 09:27
Autor Wiadomość
Pralkarz
Temat założony przez niniejszego użytkownika
Losowanie bez powtórzeń - poprawność kodu
» 2017-05-04 13:33:14
Druga część zadania domowego. Działa i spełnia swoją funkcję, ale mam wrażenie, że trzeba było to zrobić znacznie krócej (obecnie 83 linijki). Zostawić to tak jak jest czy jednak coś powinienem zmienić? Pytam głównie dlatego, że słyszałem, żeby już na początku nauki programowania dbać o czysty, jak najkrótszy kod.

C/C++
#include <iostream>
#include <cstdlib>
#include <ctime>

void pobierzLiczby( int pob[] )
{
    int i = 0;
    int liczba;
   
    do
    {
        do
        {
            std::cout << "Wprowadz " << i + 1 << ". liczbe (co najmniej siedem roznych): ";
            std::cin.clear();
            std::cin.sync();
            std::cin >> liczba;
            pob[ i ] = liczba;
        } while( std::cin.fail() );
       
        i++;
    } while( i < 10 );
   
}

bool czyWylosowana( int wyl[], int liczba )
{
    int i = 0;
   
    do
    {
        if( liczba == wyl[ i ] )
             return true;
        else
             i++;
       
    } while( i < 8 );
   
    return false;
}

int losuj( int pob[] )
{
    int indeks =( rand() % 10 );
   
    return pob[ indeks ];
}

int main()
{
    srand( time( NULL ) );
   
    int pobrane[ 10 ];
    int wylosowane[ 8 ];
   
    pobierzLiczby( pobrane );
   
    int i = 0;
    int liczba;
   
   
    do
    {
        do
        {
            liczba = losuj( pobrane );
        } while( czyWylosowana( wylosowane, liczba ) );
       
        wylosowane[ i ] = liczba;
       
        i++;
    } while( i < 8 );
   
    i = 0;
   
    std::cout << std::endl << "Wylosowane: " << std::endl;
   
    do
    {
        std::cout << wylosowane[ i ] << std::endl;
        i++;
    } while( i < 8 );
   
    return 0;
}
P-160698
karambaHZP
» 2017-05-04 20:34:04
dbać o czysty, jak najkrótszy kod.
Kod powinien być przejrzysty i prosty w zrozumieniu.
Nie musi być krótki, aby był dobry.
To jest krótsza wersja twojego kodu, ale czy lepsza? Tego nie wiem. Pewnie wolniejsza.
C/C++
#include <iostream>
#include <algorithm>
#include <random>
#include <chrono>
#include <vector>
#include <set>

int main()
{
    std::size_t n_nums;
    std::cin >> n_nums; // ile ma być liczb
    std::set < int > tmp_cont; // kontener przechowujący dane bez powtórzeń
    while( tmp_cont.size() < n_nums ) { // wstawiaj póki nie osiągnie rozmiaru
        int tmp;
        std::cin >> tmp;
        tmp_cont.insert( tmp );
    }
    std::vector < int > nums( n_nums ); // tablica dynamiczna o rozmiarze n_nums
    std::copy( tmp_cont.cbegin(), tmp_cont.cend(), nums.begin() ); // kopiowanie tmp_cont od tablicy nums
    std::shuffle( nums.begin(), nums.end(), // mieszanie zawartości tablicy
    std::default_random_engine( std::chrono::system_clock::now().time_since_epoch().count() ) );
    std::size_t n_rand;
    std::cin >> n_rand;
    if( n_rand <= n_nums ) {
        for( std::size_t i { }; i < n_rand; ++i ) { // wypisanie n_rand liczb z pomieszanej tablicy
            std::cout << nums[ i ] << ' ';
        }
    }
}

edit:
ew można wczytać dane do kontenera mieszającego i wypisać pierwszych n liczb:
C/C++
#include <iostream>
#include <boost/functional/hash.hpp>
#include <unordered_set>
#include <chrono>
#include <random>
#include <limits>

std::mt19937 gen( std::random_device { }() );
std::uniform_int_distribution < std::size_t > dist( 0, std::numeric_limits < size_t >::max() );

template < class type >
struct my_hash {
    std::size_t operator ()( type const & val ) const
    {
        std::size_t seed = dist( gen );
        boost::hash_combine( seed, val );
        return seed;
    }
};

int main()
{
    std::unordered_set < int, my_hash < int >> nums;
    std::size_t n_nums;
    std::cin >> n_nums;
    while( nums.size() < n_nums ) {
        int tmp;
        std::cin >> tmp;
        nums.insert( tmp );
    }
    std::size_t n_rand;
    std::cin >> n_rand;
    if( n_rand < n_nums ) {
        auto it = nums.cbegin();
        for( std::size_t i { }; i < n_rand; ++i ) {
            std::cout << * it++ << ' ';
        }
    }
}
P-160705
Elaine
» 2017-05-04 22:29:17
Ten drugi kod ma niezdefiniowane zachowanie (23.2.5/5):
Two values k1 and k2 of type Key are considered equivalent if the container’s key equality predicate returns true when passed those values. If k1 and k2 are equivalent, the container’s hash function shall return the same value for both.
P-160707
karambaHZP
» 2017-05-04 23:17:32
@Alueril: myślałem, że wystarczy domyślny
operator ==
.
C/C++
// ...
template < class type >
struct my_cmp {
    bool operator ()( type const & lhs, type const & rhs ) const
    {
        return lhs == rhs;
    }
};
// ....
std::unordered_set < int, my_hash < int >, my_cmp < int >> nums;
Uszanowanie za uwagę.
P-160709
Elaine
» 2017-05-05 04:22:10
To nic nie zmienia. Jeśli dwa elementy są równe według komparatora, to ich hasze muszą być równe.
P-160713
karambaHZP
» 2017-05-05 08:03:47
Czy chodzi o samego
seed
a?
Zadbać, żeby do
hash_combine
 trafiał ten sam
seed
 dla takiej samej
val
?
P-160714
Pralkarz
Temat założony przez niniejszego użytkownika
» 2017-05-05 09:27:38
Nic nie zrozumiałem z obu wersji Twojego kodu, ale dzięki za chęci. :v
Można zamknąć.
P-160715
« 1 »
  Strona 1 z 1