Wątek przeniesiony 2018-09-18 19:31 z C/C++ przez kq.

Ocena kodu programu do logowania i rejestracji

0

Witam, swoją przygode z c++ rozpocząłem jakies 5 dni temu, w tym czasie napisałem "program" do logowania oraz rejestracji. Prosiłbym o ocene kodu i o jakąś rade, co dalej.

*kalkulator jest w innym pliku podpiętym wiec go tutaj nie ma.


#include "pch.h"
#include <iostream>
#include <string>
#include <fstream>
#include <windows.h>
using namespace std;
 
 
bool IsLoggedIn()
{
    string UserLogin, UserPassword, login, password;
 
    cout << "Podaj Login: "; cin >> UserLogin;
    cout << "Podaj Haslo: "; cin >> UserPassword;
    ifstream read("C:\\Users\\user\\PROGRAMY\\C++Program\\baza\\" + UserLogin + ".txt");
    getline(read, login);
    getline(read, password);
 
    if (login == UserLogin && password == UserPassword)
    {
        return true;
    }
    else
    {
        return false;
    }
 
 
 
}
int menu()
{
    int choice2;
    {
        Sleep(1000);
        system("cls");
        cout << "_______MENU GLOWNE_______" << endl;
        cout << "1. Kalkulator" << endl;
        cout << "2. xxxx" << endl;
        cout << "3. xxxxx" << endl;
        cout << "4. xxxxxx" << endl;
        cout << "5. Wyjscie" << endl;
        cout << "WYBOR: ";
        cin >> choice2;
        switch (choice2)
        {
        case 1:
        {
            cout << "Wybrales kalkulator" << endl;
            kalkulator();
        }
        break;
 
        case 2:
        {
            cout << "FUNKCJA W BUDOWIE." << endl;
            menu();
        }
        break;
 
        case 3:
        {
            cout << "FUNKCJA W BUDOWIE." << endl;
            menu();
        }
        break;
 
        case 4:
        {
            cout << "FUNKCJA W BUDOWIE." << endl;
            menu();
        }
        break;
        case 5:
        {
            exit(0);
        }
 
        default: cout << "Nie ma takiej opcji." << endl;
        }
 
    }
    exit(0);
}
int main()
{
    int choice;
 
    cout << "___________START_____________\n";
    cout << "1. Rejestracja\n2. Login\nTwoj Wybor: "; cin >> choice;
 
 
    if (choice == 1)
    {
        string login, password, repassword, UserLogin;
 
 
        cout << "Wybierz login: "; cin >> login;
        cout << "Wpisz haslo: "; cin >> password;
        cout << "Wpisz ponownie haslo: "; cin >> repassword;
         
        ifstream read("C:\\Users\\user\\PROGRAMY\\C++Program\\baza\\" + login + ".txt");
        getline(read, UserLogin);
        while (login == UserLogin)
        {
            cout << "\nUzytkownik o tej nazwie juz istnieje" << endl;
            cout << "Sprobuj ponownie." << endl;
            cout << "Wybierz login: "; cin >> login;
            cout << endl;     
                         
        }
        while (repassword != password)
        {
            cout << endl << "Hasla nie zgadzaja sie!\nSprobuj ponownie." << endl << endl;
            cout << "Wpisz haslo: "; cin >> password;
            cout << "Potworz haslo: "; cin >> repassword;
 
        }
        while (repassword == password)
        {
            Sleep(1000);
            system("cls");
            cout << "Rejestracja zostala zakonczona!\n Nastepuje przeniesienie do  menu glownego!";
            Sleep(1000);
            system("cls");
            ofstream file;
            file.open("C:\\Users\\user\\PROGRAMY\\C++Program\\baza\\" + login + ".txt");
            file << login << endl;
            file << password;
            file.close();
            main();
                 
        }
 
    }
    else if (choice == 2)
    {
        bool status = IsLoggedIn();
 
        if (!status)
        {
            cout << "Zly login" << endl;
            main();
 
        }
        else
        {
            cout << "Pomyslne logowanie" << endl;
            menu();
        }
     
     
 
     
 
    }
    else
    {
        cout << "Nie ma takiego wyboru." << endl;
        system("PAUSE");
        return 0;
    }
 
 
    return 0;
 
}


0

Przez pięć dni ogarnąłęś nawet preprocesor oraz wejście i wyjście plikowe? A spałeś coś?

0

@lamerski: Szkoła się zaczęła, więc po 3/4h dziennie jakoś :D
/// A tak na poważnie, opanowanie tego zajęło mi jakieś 4/5h.

3

Na pewno przydałoby się podzielić funkcję main na mniejsze części, zachowując pojedynczą odpowiedzialność. Funkcja IsLoggedIn też robi kilka rzeczy, a nie powinna. Przy czym określanie wartości zwracanej jest za długie:

if (login == UserLogin && password == UserPassword)
{
    return true;
}
else
{
    return false;
}

bo wystarczy tyle:

return (login == UserLogin && password == UserPassword);

Problem pojawia się też tutaj:

file.open("C:\\Users\\user\\PROGRAMY\\C++Program\\baza\\" + login + ".txt");

Hardkodowanie ścieżek do plików zawsze jest złe. Gdybym spróbował uruchomić Twój program u siebie, to by mi błędy wyskoczyły, bo nie posiadam ani decolowego pliku, a tym bardziej katalogu o takiej ścieżce.


lamerski napisał(a):

Przez pięć dni ogarnąłęś nawet preprocesor oraz wejście i wyjście plikowe? A spałeś coś?

Umieć wykorzystać dany kod, a go znać i rozumieć, to dwie różne rzeczy. ;)

0

@furious programming: "Funkcja IsLoggedIn też robi kilka rzeczy, a nie powinna" O co Ci tutaj dokładnie chodzi, bo nie rozumiem. Co do podzielenia maina zajmę się tym jak wrócę ze szkoły

0

@zimny.: poczytaj o SOLID

3

@fasadin: Większość pracujących jako programiści nie stosuje się do SOLID a ty chcesz by 5cio dniowy newbie używał :). Ale z drugiej strony gość się dobrze zapowiada, więc w sumie słusznie.

@zimny. od zera 5 dniach coś takiego, od razu widać, że będą z ciebie ludzie. (jest tu wielu co kodzą od roku i tworzą dużo gorszy kod).
Nie zmienia to faktu, że jeszcze długa droga przed tobą.

Są pewne proste zasady, które pomagają pisać dobry kod:

  1. pisz małe funkcje, które mają zaledwie parę linijek maksymalnie dwie instrukcje sterujące if for switch while (ułatwia to utrzymanie "Single responsibility principle"
  2. dobrze nazywaj zmienne, funkcje powinny być czasownikami, zmienne rzeczownikami (ta twoja zmienna read to proszenie się o kłopoty) - zawsze po angielsku
  3. nie stosuj liczb magicznych
  4. Trzymaj się jakiegoś standardu kodowania: gdzie mają być nawiasy, klamry itp, jak głębokie mają być wcięcia, jakimi literami są opisane sybole (małe/duże), (cammel case).

Jest jeszcze parę innych zasad, ale na razie to ci wystarczy.

Co do kodu:

  • bool IsLoggedIn()
    *- zmienna read nie dość, że to czasownik, to zbyt powszechnie używany
    *- powinno, być bool verifyUser(std::string &userName, std::string &password)
  • int menu()
    *- rekursja zupełnie nie ma tutaj sensu, menu powinno być wywoływane w pętli
    *- exit(0) na końcu do wyrzucenia
  • funkcja main
    *- za długie i skomplikowane (podziel na funkcje)

Sleep to jest coś czego się nie powinno używać.

Naucz się posługiwać debuggerem, to powinien być teraz priorytet, bo bardzo ułatwia zrozumienie, co się dzieje w kodzie.
Podstawy typu step over, step into, run to i breakpoints zupełnie wystarczą.

offtopic: ten markdown działa jak chce :(

0

@MarekR22: Dziękuję za rady, za klika dni postaram się wysłać coś nowego, niestety szkoła i nie mam za wiele czasu :x
Podsumowując Wasze odpowiedzi:

  1. Dzielić programy na funkcje,
  2. Używać mniej powszechnych czasowników,
  3. Wyrzucic sleep
  4. Pisać bardziej "przejrzyście",
  5. Ścieżki do plików muszą być "uniwersalne.
    // Co to tego IsLoogedIn - mógłbyś mi pokazać jak to ma wyglądać prawidłowo?
1
zimny. napisał(a):
  1. Dzielić programy na funkcje,

Dzielić programy na małe fragmenty, gdzie każdy fragment (np. funkcja) odpowiada tylko za jedną rzecz. Z takich małych, uniwersalnych klocków o wiele łatwiej budować większe programy i je później rozwijać.

  1. Używać mniej powszechnych czasowników,

Nie o to chodzi. Powinieneś używać słów jak najbardziej popularnych, jednak do nazywania zmiennych zawsze używać rzeczowników, a do funkcji zawsze używać czasowników.

  1. Wyrzucic sleep

To zależy. Jeśli koniecznie potrzebujesz wstrzymać działanie programu na pewien czas, całkowicie blokując jej działanie (i przy okazji nie obciążając procesora), to sleep właśnie do tego służy. Jednak jeśli programu wstrzymywać nie potrzebujesz, to nie ma sensu go używać.

  1. Pisać bardziej "przejrzyście",

Ogólnie chodzi o to, aby zapoznać się z przyjętą konwencją nazewnictwa oraz stosować powszechnie ustalony sposób formatowania kodu, który m.in. dotyczy umiejscowienia klamer oraz wcięć w kodzie. Z tymi zasadami powinieneś się zapoznać już teraz, aby od początku przygody z programowaniem wpajać sobie tylko dobre nawyki.

  1. Ścieżki do plików muszą być "uniwersalne.

Być może jeszcze za wcześnie na to. Nie wiem co pod tym kątem jest dostępne w bibliotece standardowej, jednak ścieżki katalogów specjalnych pobiera się pośrednio lub bezpośrednio za pomocą funkcji systemowych. Można też korzystać ze ścieżek relatywnych, jednak najpierw trzeba by wiedzieć co nieco na temat uprawnień i co można zrobić w danej lokalizacji na dysku.

To może zostaw sobie na przyszłość, aby nie mieszać sobie w głowie.

0

Całkowicie usunąłem "Sleep", funkcja main ma teraz o dużo mniejszy kod, rozłożyłem go na funkcje tak jak mówiliście.
main.cpp:


#include "pch.h"
#include <iostream>
#include <string>
#include <fstream>
#include <windows.h>
using namespace std;


bool verifyUser()
{
	string UserLogin, UserPassword, login, password;

	cout << "Podaj Login: "; cin >> UserLogin;
	cout << "Podaj Haslo: "; cin >> UserPassword;
	ifstream get("C:\\Users\\user\\PROGRAMY\\C++Program\\baza\\" + UserLogin + ".txt");
	getline(get, login);
	getline(get, password);

	return (login == UserLogin && password == UserPassword);

}
int main()
{
	int choice;

	cout << "___________START_____________\n";
	cout << "1. Rejestracja\n2. Login\nTwoj Wybor: "; cin >> choice;

	if (choice == 1)
	{
		cout << "Wybrales rejestracje." << endl;
		registration();

	}
	else if (choice == 2)
	{
		bool status = verifyUser();

		if (!status)
		{
			system("cls");
			cout << "Zly login\n";
			main();

		}
		else
		{
			cout << "Pomyslne logowanie" << endl;
			menu();
		}

	}
	else 
	{
		cout << "Nie ma takiego wyboru." << endl;
		system("PAUSE");
		return 0;
	}


	return 0;

}


pch.cpp:


#include "pch.h"
#include <iostream>
#include <Windows.h>
#include <fstream>
#include <string>
#include <stdio.h>
#include <conio.h>
using namespace std;
void kalkulator()
{
	int x, y, choice2;
	
for (;;)
{

	cout << endl;
	cout << "Podaj 1 liczbe: ";
	cin >> x;
	cout << "Podaj 2 liczbe: ";
	cin >> y;
	cout << endl;
	cout << "Teraz wybierz co chcesz zrobic" << endl;
	cout << "1. Dodawanie" << endl;
	cout << "2. Odejmowanie" << endl;
	cout << "3. Mnozenie" << endl;
	cout << "4. Dzielenie" << endl;
	cout << "5. Wyjscie" << endl;
	cout << "WYBIERZ:";
	cin >> choice2;
	switch(choice2)
	{

	case 1:
	{
		cout << "________________________________________" << endl;
		cout << "Suma " << x << " i " << y << " = " << x + y << endl;
		cout << "________________________________________" << endl;
	}
	break;

	case 2:
	{
		cout << "________________________________________" << endl;
		cout << "Roznica " << x << " i " << y << " = " << x - y << endl;
		cout << "________________________________________" << endl;
	}
	break;

	case 3:
	{
		cout << "________________________________________" << endl;
		cout << "Iloczyn " << x << " i " << y << " = " << x * y << endl;
		cout << "________________________________________" << endl;
	}
	break;

	case 4:
	{
		if (y == 0) cout << "Nie dzielimy przez 0!";
		else
		{
			cout << "________________________________________" << endl;
			cout << "Iloraz " << x << " i " << y << " = " << x / y << endl;
			cout << "________________________________________" << endl;
		}
	}
	break;
	case 5:
	{
		exit(0);
	}
	default: cout << "Nie ma takiej opcji.";
	}

}

}
void registration()
{
	string login, password, repassword, UserLogin;


	cout << "Wybierz login: "; cin >> login;
	cout << "Wpisz haslo: "; cin >> password;
	cout << "Wpisz ponownie haslo: "; cin >> repassword;

	ifstream get("C:\\Users\\user\\PROGRAMY\\C++Program\\baza\\" + login + ".txt");
	getline(get, UserLogin);
	while (login == UserLogin)
	{
		cout << "\nUzytkownik o tej nazwie juz istnieje" << endl;
		cout << "Sprobuj ponownie." << endl;
		cout << "Wybierz login: "; cin >> login;
		cout << endl;

	}
	while (repassword != password)
	{
		cout << endl << "Hasla nie zgadzaja sie!\nSprobuj ponownie." << endl << endl;
		cout << "Wpisz haslo: "; cin >> password;
		cout << "Potworz haslo: "; cin >> repassword;

	}
	while (repassword == password)
	{
		system("cls");
		cout << "Rejestracja zostala zakonczona!\n Nastepuje przeniesienie do  menu glownego!";
		system("cls");
		ofstream file;
		file.open("C:\\Users\\user\\PROGRAMY\\C++Program\\baza\\" + login + ".txt");
		file << login << endl;
		file << password;
		file.close();
		menu();

	}
}
void menu()
{
	{
		int choice2;
		{
		
			system("cls");
			cout << "_______MENU GLOWNE_______" << endl;
			cout << "1. Kalkulator" << endl;
			cout << "2. xxxx" << endl;
			cout << "3. xxxxx" << endl;
			cout << "4. xxxxxx" << endl;
			cout << "5. Wyjscie" << endl;
			cout << "WYBOR: ";
			cin >> choice2;
			switch (choice2)
			{
			case 1:
			{
				cout << "Wybrales kalkulator" << endl;
				kalkulator();
			}
			break;

			case 2:
			{
				cout << "FUNKCJA W BUDOWIE." << endl;
				registration();
			}
			break;

			case 3:
			{
				cout << "FUNKCJA W BUDOWIE." << endl;
				menu();
			}
			break;

			case 4:
			{
				cout << "FUNKCJA W BUDOWIE." << endl;
				menu();
			}
			break;
			case 5:
			{
				exit(0);
			}

			default: cout << "Nie ma takiej opcji." << endl;
			}

		}
		
	}
}


Wycinku z pch.h nie podaje, bo nie ma sensu.
Stwierdziłem że na razie nie będę zmieniał ścieżki do pliku. W przyszłości planuje podpiąć pod to bazę MySQL.

@MarekR22 "menu powinno być wywoływane w pętli" to znaczy?
Dalej nie wiem jak poprawić aktualne "verifyUser"
Metoda do sprawdzania czy dany login już istnieje jest dobra? Można to napisać w jakiś lepszy sposób?
W menu i kalkulatorze chciałem użyć "getch", lecz coś mi nie działało.

//Edit:
Zrobiłem dodawanie danych do profilu i ich odczyt. Lecz pewne rzeczy mi się nie podobają. Branie loginu i pass z pliku żeby go wypisać od nowa wraz z imieniem, nazwiskiem i telefonem. Gdy tego nie zrobie, imie i nazwisko "wchodzą" w miejsce loginu i passa. Podobna sytuacja jest z pokazywaniem danych.

void UsersAddData()
{
	string name, surname, login, password;
	string telephone;
	system("cls");
	cout << "Dodawanie danych osobistych do swojego konta" << endl;
	cout << "Imie: "; cin >> name;
	cout << "Nazwisko: "; cin >> surname;
	cout << "Telefon: "; cin >> telephone;
	cout << "Potwierdz swoja nazwe konta: "; cin >> login;
	ifstream get("C:\\Users\\user\\PROGRAMY\\C++Program\\baza\\" + login + ".txt");
	getline(get, login);
	getline(get, password);
	ofstream file;
	file.open("C:\\Users\\user\\PROGRAMY\\C++Program\\baza\\" + login + ".txt");
	file << login << endl;
	file << password << endl;
	file << name << endl;
	file << surname << endl;
	file << telephone << endl;
	file.close();
	menu();

}
void ShowData()
{
	string name, surname, login, telephone;
	string password, UserPassword;
	

	cout << "Aby wyswietlić swoje dane wpisz login: "; cin >> login;
	ifstream get("C:\\Users\\user\\PROGRAMY\\C++Program\\baza\\" + login + ".txt");
	getline(get, login);
	getline(get, password);
	getline(get, name);
	getline(get, surname);
	getline(get, telephone);
	cout << "Dane uzytkownika: " << login << endl;
	cout << "Imie: " << name << endl;
	cout << "Nazwisko: " << surname << endl;
	cout << "Nr telefonu: " << telephone << endl;
	

}

1 użytkowników online, w tym zalogowanych: 0, gości: 1