Ocena kodu źródłowego

pytanie zadane 22 czerwca 2018 w C i C++ przez Huberti Gaduła (4,500 p.)

Dzień dobry, chciałbym prosić o ocenę kodu.

Program ma za zadanie pobrać od użytkownika maksymalną długość słowa oraz słowo. Następnie skrócić podane słowo do maksymalnej długości według poniższego algorytmu:

napisz nazwę zmiennej w postaci, w której życzyłbyś sobie ją widzieć – używaj tylko liter, cyfr oraz znaków '_' (podkreślenie) i '$' (dolar); może zabrzmi to dziwnie, ale znak '$' traktować będziemy jak literę (czyni to do dzisiaj wiele kompilatorów);

jeśli długość nazwy jest mniejsza bądź równa n, możesz jej użyć i nie musisz robić nic więcej

w przeciwnym wypadku usuwaj z nazwy, począwszy od końca, wszystkie znaki, które nie są literami i cyframi – w chwili, w której długość nazwy osiągnie n, możesz zakończyć pracę i użyć nazwy zmiennej

jeśli długość nazwy nadal jest większa od n, usuwaj z niej, począwszy od końca, kolejne cyfry - w chwili, w której długość nazwy osiągnie n możesz zakończyć pracę i użyć nazwy zmiennej

jeśli długość nazwy nadal jest większa od n, usuwaj z niej, począwszy od początku, kolejne samogłoski z wyjątkiem pierwszej (chodzi o to, by w nazwie została chociaż jedna samogłoska, o ile jakakolwiek została użyta) - w chwili, w której długość nazwy osiągnie n, możesz zakończyć pracę i użyć nazwy zmiennej

jeśli długość nazwy nadal jest większa od n, usuwaj z niej znaki od końca, począwszy od przedostatniego - w chwili, w której długość nazwy osiągnie n, możesz zakończyć pracę i użyć nazwy zmiennej

Github:

https://github.com/huberti1/NameShortener.git

Kod źródłowy:

#include <iostream>
#include <string>

struct Name
{
	std::string name;
	int sizeMaximum = 0;
};

int getTestsResult();
bool testNameSizeLowerThenMaximumSize();
bool testNameSizeEquelMaximumSize();
bool testTooManyAllOthers();
bool testTooManyPartOthers();
bool testTooManyAllDigits();
bool testTooManyPartDigits();
bool testTooManyAllVowels();
bool testTooManyPartVowels();
bool testTooManyAllWithoutVowel();
bool testTooManyAll();

Name setupShortener();
std::string shortenToSizeMaximum(Name name);
bool isSizeMaximumExceeded(const Name & name);
std::string excludeUnnecessaryOthers(const Name & name);
int calculateAmountNecessaryOthers(const Name & name);
int calculateAmountWithoutOthers(const Name & name);
bool isOther(char source);
bool isDollar(char source);
std::string excludeUnnecessaryDigits(const Name & name);
int calculateAmountNecessaryDigits(const Name & name);
int calculateAmountWithoutDigits(const Name & name);
bool isThereAnyVowel(const Name & name);
bool isVowel(char source);
std::string excludeUnnecessaryVowelsStartFromSecoundVowel(const Name & name);
std::string getEverythingToFirstVowelInclusive(const Name & name);
std::string getEverythingFromOneAfterFirstVowelExcludingUnnecessaryVowels(const Name & name);
size_t getFirstVowelIndex(const Name & name);
int calculateAmountNotLeavingVowels(const Name & name);
std::string excludeFromOneBehindLast(const Name & name);
void show(const Name & name);

int main()
{
	return getTestsResult();	

	Name name = setupShortener();	
	name.name = shortenToSizeMaximum(name);
	show(name);
	return 0;
}

Name setupShortener()
{
	Name name;

	std::cin >> name.sizeMaximum;
	std::cin.get();
	getline(std::cin, name.name);
	return name;
}

std::string shortenToSizeMaximum(Name name)//transfer by value give a transformation look
{
	if (isSizeMaximumExceeded(name))
	{
		name.name = excludeUnnecessaryOthers(name);
		if (isSizeMaximumExceeded(name))
		{
			name.name = excludeUnnecessaryDigits(name);
			if (isSizeMaximumExceeded(name))
			{
				name.name = excludeUnnecessaryVowelsStartFromSecoundVowel(name);
				if (isSizeMaximumExceeded(name))
					name.name = excludeFromOneBehindLast(name);
			}
		}
	}
	return name.name;
}

bool isSizeMaximumExceeded(const Name & name)
{
	return name.name.size() > name.sizeMaximum;
}

std::string excludeUnnecessaryOthers(const Name & name)
{
	std::string destination;
	int amountNecessaryOthers = calculateAmountNecessaryOthers(name);

	for (size_t i = 0; i < name.name.size(); i++)
	{
		if(!isOther(name.name[i]))
			destination += name.name[i];
		else if (amountNecessaryOthers > 0)
		{
			destination += name.name[i];
			amountNecessaryOthers--;
		}
	}
	return destination;
}

int calculateAmountNecessaryOthers(const Name & name)
{
	int amountNecessaryOthers = name.sizeMaximum - calculateAmountWithoutOthers(name);
	
	return amountNecessaryOthers;
}

int calculateAmountWithoutOthers(const Name & name)
{
	int amountWithoutOthers = 0;

	for (size_t i = 0; i < name.name.size(); i++)
		if (!isOther(name.name[i]))
			amountWithoutOthers++;
	return amountWithoutOthers;
}

bool isOther(char source)
{
	return !isalnum(source) && !isDollar(source);
}

bool isDollar(char source)
{
	return source == '$';
}

std::string excludeUnnecessaryDigits(const Name & name)
{
	std::string destination;
	int amountNecessaryDigits = calculateAmountNecessaryDigits(name);

	for (unsigned i = 0; i < name.name.size(); i++)
	{
		if(!isdigit(name.name[i]))
			destination += name.name[i];
		else if (amountNecessaryDigits > 0)
		{
			destination += name.name[i];
			amountNecessaryDigits--;
		}
	}
	return destination;
}

int calculateAmountNecessaryDigits(const Name & name)
{
	int amountNecessaryDigits = name.sizeMaximum - calculateAmountWithoutDigits(name);

	return amountNecessaryDigits;
}

int calculateAmountWithoutDigits(const Name & name)
{
	int amountWithoutDigits = 0;

	for (size_t i = 0; i < name.name.size(); i++)
		if (!isdigit(name.name[i]))
			amountWithoutDigits++;
	return amountWithoutDigits;
}

std::string excludeUnnecessaryVowelsStartFromSecoundVowel(const Name & name)
{
	if (isThereAnyVowel(name))
		return getEverythingToFirstVowelInclusive(name) + getEverythingFromOneAfterFirstVowelExcludingUnnecessaryVowels(name);
	else 
		return name.name;
}

bool isThereAnyVowel(const Name & name)
{
	for (size_t i = 0; i < name.name.size(); i++)
		if (isVowel(name.name[i]))
			return true;
	return false;
}

std::string getEverythingToFirstVowelInclusive(const Name & name)
{
	std::string destination;
	size_t firstVowelIndex = getFirstVowelIndex(name);

	for (size_t i = 0; i <= firstVowelIndex; i++)
		destination += name.name[i];
	return destination;
}

std::string getEverythingFromOneAfterFirstVowelExcludingUnnecessaryVowels(const Name & name)
{
	std::string destination;
	int amountNotLeavingVowels = calculateAmountNotLeavingVowels(name);
	const size_t oneAfterFirstVowel = getFirstVowelIndex(name) + 1;

	for (size_t i = oneAfterFirstVowel; i < name.name.size(); i++)
	{
		if (!isVowel(name.name[i]))
			destination += name.name[i];
		else if (amountNotLeavingVowels > 0)
			amountNotLeavingVowels--;
		else
			destination += name.name[i];
	}
	return destination;
}

int calculateAmountNotLeavingVowels(const Name & name)
{
	return name.name.size() - name.sizeMaximum;
}

size_t getFirstVowelIndex(const Name & name)
{
	size_t i = 0;

	while (!isVowel(name.name[i]))
		i++;
	return i;
}

bool isVowel(char source)
{
	return	source == 'a' || source == 'e' || source == 'i' || source == 'o' ||
		source == 'u' || source == 'y' || source == 'A' || source == 'E' ||
		source == 'I' || source == 'O' || source == 'U' || source == 'Y';
}

std::string excludeFromOneBehindLast(const Name & name)
{
	std::string destination;
	int amountLeavingCharactersWithoutLast = name.sizeMaximum - 1;

	for (int i = 0; amountLeavingCharactersWithoutLast > 0; i++)
	{
		destination += name.name[i];
		amountLeavingCharactersWithoutLast--;
	}
	destination += name.name.back();
	return destination;
}

void show(const Name & name)
{
	std::cout << name.name << std::endl;
}

int getTestsResult()
{
	if (!testNameSizeLowerThenMaximumSize())
		return 1;
	if (!testNameSizeEquelMaximumSize())
		return 2;
	if (!testTooManyAllOthers())
		return 3;
	if (!testTooManyPartOthers())
		return 4;
	if (!testTooManyAllDigits())
		return 5;
	if (!testTooManyPartDigits())
		return 6;
	if (!testTooManyAllVowels())
		return 7;
	if (!testTooManyPartVowels())
		return 8;
	if (!testTooManyAllWithoutVowel())
		return 9;
	if (!testTooManyAll())
		return 10;
	return 0;
}

bool testNameSizeLowerThenMaximumSize()
{
	Name name{ "AB$ 1", 6 };
	std::string expected = "AB$ 1";

	name.name = shortenToSizeMaximum(name);

	return name.name == expected;
}

bool testNameSizeEquelMaximumSize()
{
	Name name{ "AB$ 1", 5 };
	std::string expected = "AB$ 1";

	name.name = shortenToSizeMaximum(name);

	return name.name == expected;
}

bool testTooManyAllOthers()
{
	Name name{ "A_ B$1", 4 };
	std::string expected = "AB$1";

	name.name = shortenToSizeMaximum(name);

	return name.name == expected;
}

bool testTooManyPartOthers()
{
	Name name{ "A_ __ _B$1", 6 };
	std::string expected = "A_ B$1";

	name.name = shortenToSizeMaximum(name);

	return name.name == expected;
}

bool testTooManyAllDigits()
{
	Name name{ "A1B$123456789", 3 };
	std::string expected = "AB$";

	name.name = shortenToSizeMaximum(name);

	return name.name == expected;
}

bool testTooManyPartDigits()
{
	Name name{ "3A123B456$789", 4 };
	std::string expected = "3AB$";

	name.name = shortenToSizeMaximum(name);

	return name.name == expected;
}

bool testTooManyAllVowels()
{
	Name name{ "AAEIOUY$aeioDuy", 3 };
	std::string expected = "A$D";

	name.name = shortenToSizeMaximum(name);

	return name.name == expected;
}

bool testTooManyPartVowels()
{
	Name name{ "A$aeiouyBAEIOUY", 5 };
	std::string expected = "A$BUY";

	name.name = shortenToSizeMaximum(name);

	return name.name == expected;
}

bool testTooManyAllWithoutVowel()
{
	Name name{ "B$BE", 3 };
	std::string expected = "B$E";

	name.name = shortenToSizeMaximum(name);

	return name.name == expected;
}

bool testTooManyAll()
{
	Name name{ "B$ACB", 4 };
	std::string expected = "B$AB";

	name.name = shortenToSizeMaximum(name);

	return name.name == expected;
}

2 odpowiedzi

odpowiedź 23 czerwca 2018 przez RafalS VIP (122,820 p.)
wybrane 3 lipca 2018 przez Huberti

Najlepsza

Dlugi ten kod wiec pisalem komentarze w kodzie.

Generalnie podsumowujac:

duzy minus za napisanie tego strukturalnie w C++, jak juz piszesz w C++ to pisz obiektowo
duzy minus za wrzucenie wszystkiego w jeden plik
duzy plus za napisanie testow i duzy minus za trzymanie ich w tym samym pliku, wywolanie w mainie i nie uzywanie gotowych frameworkow do testowania kodu C++
wedlug mnie minus za przekombinowaną implementacje, gdybys to napisal dokladnie tak jak jest w tekscie to program bylby czytelniejszy i krotszy
minus za wywarzanie drzwi od nowa, napisalem tam w kilku miejscach jak to uproscic
plus za rozbicie na funkcje
generalnie plus za robienie dlugich nazw zmiennych, ale minus ze nie zawsze byly trafne i czasem niewiele mowily

A tutaj masz wszystkie komentarze:

#include "stdafx.h"

#include <iostream>
#include <algorithm>
#include <string>

//duzy minus za wrzucenie wszystkiego w jeden plik

//plus za polaczenie imienia z maksimum w strukturze
//minus za napisanie tego proceduralnie
//czyli plus za polaczenie danych razem
//minus za odsperowanie zachowania od danych
//funkcje sa odzielone od danych na których operuja
//w podejsciu obiektowym wiekszosc z tych funkcji byłaby
//metodami klasy i nie przesyłałbyś do każdej funkcji
//const Name& name

//nie czepialbym sie gdybys pisal w C, ale piszesz w C++
//wiec pisz obiektowo :P

//za bardzo ogolna nazwa struktury
//gdyby ktos chcial wykorzystac Twoj kod jako biblioteke
//w swoim kodzie to nazwa typu Name posrod calej reszty
//struktur z ktorych by korzystal bylaby niejasne
//tutaj nie jest, bo jest jedyna struktura
struct Name
{
	std::string name;
	//ja bym zamienil - maxSize
	//czesciej sie z takimi nazwami spotykam :P
	int sizeMaximum = 0;
};

//duzy plus za pisanie testow
//oraz duzy minus za umieszczenie ich w jednym
//pliku z cala reszta i nie uzywanie frameworkow
//do testowania

//a teraz napisze czego spodziewam sie po tych nazwach
//a potem zweryfikujemy czy nazwy byly dobrze dobrane

//get sugeruje ze funkcja nie robi nic poza zwroceniem
//test result, a prawdopodobnie puszcza ona wszystkie testy
//druga sprawa co to jest testResult?
//liczba testow ktore przeszly? 1 lub 0?
//dla mnie to jest niejasne
//generalnie takich funkcji sie nie tworzy bo testy pisze
//sie we frameworkach
int getTestsResult();
//literowka w Then :P
//ale generalnie bardzo dobra nazwa
//dobrze opisuje testowany przypadek
bool testNameSizeLowerThenMaximumSize();
//literowka w Equel
bool testNameSizeEquelMaximumSize();
//totalnie nie mam pojecia co to moze testowac
//za duzo wszystkich innych, co to za przypadek testowy?
bool testTooManyAllOthers();
//to samo
bool testTooManyPartOthers();
bool testTooManyAllDigits();
//part digits? czyli ze troche liter troche czegokolwiek?
bool testTooManyPartDigits();
bool testTooManyAllVowels();
bool testTooManyPartVowels();
bool testTooManyAllWithoutVowel();
bool testTooManyAll();
//jak przeczytalem wszystkie to juz czaje co to bedzie
//testowac, trzymajac sie Twojej konwencji ciezko to nazwac
//ale np testNameTooLongWithDigitsOnly
// testNameTooLongWithSomeDigits
//a ogolne testTooManyAll po prostu
//testNameTooLong, skoro moze zawierac cokolwiek to
//nie ma zadnego ograniczenia wiec nie jest to 
//specyficzny tylko normalny przypadek :P


//nie mamy klasy shortener, wiec czemu robimy jej setup
//zielonego pojecia co to moze robic
//zwraca Name, ale jak tworzy to name, na jakiej podstawie?
//skad bierze dane (pewnie cinem :D, ale trzeba to zaznaczyć)
Name setupShortener();
//shortenToMaximumSize?
std::string shortenToSizeMaximum(Name name);
//isMaximumSizeExceeded albo
//isMaxSizeExceeded
//czepiam sie z tymi nazwami :D, generalnie nie sa zle
//ale przymiotnik po rzeczowniku mi tu dziwnie wyglada
//w kazdym kodzie jaki czytalem bylo na odwrot
bool isSizeMaximumExceeded(const Name & name);
//czym sa others, zwroci name z Name bez tych others
//czy tylko je?
std::string excludeUnnecessaryOthers(const Name & name);
//necessary do czego, po co calculate amount
//czemu nie count
int calculateAmountNecessaryOthers(const Name & name);
int calculateAmountWithoutOthers(const Name & name);
bool isOther(char source);
bool isDollar(char source);
std::string excludeUnnecessaryDigits(const Name & name);
int calculateAmountNecessaryDigits(const Name & name);
int calculateAmountWithoutDigits(const Name & name);
bool isThereAnyVowel(const Name & name);
bool isVowel(char source);
std::string excludeUnnecessaryVowelsStartFromSecoundVowel(const Name & name);
std::string getEverythingToFirstVowelInclusive(const Name & name);
std::string getEverythingFromOneAfterFirstVowelExcludingUnnecessaryVowels(const Name & name);
size_t getFirstVowelIndex(const Name & name);
int calculateAmountNotLeavingVowels(const Name & name);
std::string excludeFromOneBehindLast(const Name & name);
void show(const Name & name);

int main()
{
	//to jest bardzo brzydki sposob
	//puszczania testow :D
	//dlatego jest tyle gotowych frameworkow
	//do ich pisania
	std::cout << getTestsResult() << std::endl;

	Name name = setupShortener();
	std::cout << excludeFromOneBehindLast(name) << std::endl;
	name.name = shortenToSizeMaximum(name);
	show(name);
	return 0;
}

//zbyt ogolna nazwa funkcji, po samej sygnaturze
//nie wiem co ona bedzie robic - musze analizować ciało :(
Name setupShortener()
{
	Name name;

	std::cin >> name.sizeMaximum;
	std::cin.get();
	getline(std::cin, name.name);
	return name;
}

std::string shortenToSizeMaximum(Name name)//transfer by value give a transformation look
{
	//za duze te zagniezdzenia
	//tzw arrow code
	//odwroc warunki a uzyskamy ladny płaski kod bez
	//zagniezdzen
	//ale generalnie duzy plus za rozbicie tego na wiele
	//funkcji zamiast rzucenia wszystkiego do tej funkcji
	if (isSizeMaximumExceeded(name))
	{
		//hmm skoro wszedzie przypisujesz name
		//to czemu te funkcje zwracaja string a nie
		//edytuja name.name
		name.name = excludeUnnecessaryOthers(name);
		if (isSizeMaximumExceeded(name))
		{
			name.name = excludeUnnecessaryDigits(name);
			if (isSizeMaximumExceeded(name))
			{
				name.name = excludeUnnecessaryVowelsStartFromSecoundVowel(name);
				if (isSizeMaximumExceeded(name))
					name.name = excludeFromOneBehindLast(name);
			}
		}
	}
	return name.name;
}

bool isSizeMaximumExceeded(const Name & name)
{
	return name.name.size() > name.sizeMaximum;
}

std::string excludeUnnecessaryOthers(const Name & name)
{
	std::string destination;
	//amountOfNecessaryOthers?
	int amountNecessaryOthers = calculateAmountNecessaryOthers(name);

	for (size_t i = 0; i < name.name.size(); i++)
	{
		if (!isOther(name.name[i]))
			destination += name.name[i];
		else if (amountNecessaryOthers > 0)
		{
			destination += name.name[i];
			amountNecessaryOthers--;
		}
	}
	return destination;
}

int calculateAmountNecessaryOthers(const Name & name)
{
	int amountNecessaryOthers = name.sizeMaximum - calculateAmountWithoutOthers(name);

	return amountNecessaryOthers;
	//po co zmienna, czemu nie od razu return
}

int calculateAmountWithoutOthers(const Name & name)
{
	int amountWithoutOthers = 0;

	for (size_t i = 0; i < name.name.size(); i++)
		if (!isOther(name.name[i]))
			amountWithoutOthers++;
	//return amountWithoutOthers;
	//mozna uproscic algorytmami z <algorithm>:
	//mozliwe ze wyglada Ci to dziwnie i zastanawiasz sie
	//jakim cudem to jest uproszczenie
	//otoz ludzie ktorzy pracuja z tymi funkcjami na co dzien
	//szybciej zrozumieja co ten kod robi gdy zobacza
	//znajoma funkcja a nie jakis arbitralny kod :P
	return std::count_if(name.name.begin(), name.name.end(),
		[](char c) {
		return !isOther(c);
	});
}

bool isOther(char source)
{
	return !isalnum(source) && !isDollar(source);
}

bool isDollar(char source)
{
	return source == '$';
}

std::string excludeUnnecessaryDigits(const Name & name)
{
	std::string destination;
	int amountNecessaryDigits = calculateAmountNecessaryDigits(name);
	for (unsigned i = 0; i < name.name.size(); i++)
	{
		if (!isdigit(name.name[i]))
			destination += name.name[i];
		else if (amountNecessaryDigits > 0)
		{
			destination += name.name[i];
			amountNecessaryDigits--;
		}
	}
	return destination;
}

//czy mi sie wydaje czy to liczy ile cyfr trzeba zostawic?
//czemu nie:
//calculateNumberOfDigitsToLeave
//calculateNumberOfDigitsThatShouldStay
//i w takim razie czy calej tej zabawy z liczeniem ile ma zostac
//nie da sie ominac zaimplementowaniem doslownie tego co jest
//napisane w opisie algorytmu? czyli lecisz od konca i wywalasz
//dopoki wyraz za dlugi?
//na moje oko byloby o niebo bardziej przejrzyste
//jedna funkcja na kazdy punkt w opisie algorytmu:
//removeDigitsFromTheEndUntilLengthConditionMet
//podobnie dla kazdego warunku i ot caly skomplikowany algorytm
int calculateAmountNecessaryDigits(const Name & name)
{
	int amountNecessaryDigits = name.sizeMaximum - calculateAmountWithoutDigits(name);
	
	//czemu nie od razu return?
	return amountNecessaryDigits;
}

int calculateAmountWithoutDigits(const Name & name)
{
	int amountWithoutDigits = 0;
	//to samo, mozna uzyc std::count_if
	for (size_t i = 0; i < name.name.size(); i++)
		if (!isdigit(name.name[i]))
			amountWithoutDigits++;
	return amountWithoutDigits;
}

std::string excludeUnnecessaryVowelsStartFromSecoundVowel(const Name & name)
{
	if (isThereAnyVowel(name))
		return getEverythingToFirstVowelInclusive(name) + getEverythingFromOneAfterFirstVowelExcludingUnnecessaryVowels(name);
	else
		return name.name;
}

bool isThereAnyVowel(const Name & name)
{
	for (size_t i = 0; i < name.name.size(); i++)
		if (isVowel(name.name[i]))
			return true;
	return false;
}

std::string getEverythingToFirstVowelInclusive(const Name & name)
{
	std::string destination;
	size_t firstVowelIndex = getFirstVowelIndex(name);

	for (size_t i = 0; i <= firstVowelIndex; i++)
		destination += name.name[i];
	return destination;
}

std::string getEverythingFromOneAfterFirstVowelExcludingUnnecessaryVowels(const Name & name)
{
	std::string destination;
	int amountNotLeavingVowels = calculateAmountNotLeavingVowels(name);
	const size_t oneAfterFirstVowel = getFirstVowelIndex(name) + 1;

	for (size_t i = oneAfterFirstVowel; i < name.name.size(); i++)
	{
		if (!isVowel(name.name[i]))
			destination += name.name[i];
		else if (amountNotLeavingVowels > 0)
			amountNotLeavingVowels--;
		else
			destination += name.name[i];
	}
	return destination;
}

//??? po co cos takiego
//gdzie tu jest cos o samogloskach?
//przeciez liczysz tylko ile trzeba wyrzucic zeby spelnic maxsize
int calculateAmountNotLeavingVowels(const Name & name)
{
	return name.name.size() - name.sizeMaximum;
}

size_t getFirstVowelIndex(const Name & name)
{
	size_t i = 0;
	//brak sprawdzenia czy nie wyszlismy poza tablice
	while (!isVowel(name.name[i]))
		i++;
	return i;
}

bool isVowel(char source)
{
	return  source == 'a' || source == 'e' || source == 'i' || source == 'o' ||
		source == 'u' || source == 'y' || source == 'A' || source == 'E' ||
		source == 'I' || source == 'O' || source == 'U' || source == 'Y';
}

//strasznie przekombinowane musialem to kilka razy
//puscic zeby zrozumiec co to robi
//po czym okazalo sie ze to usuwa elementy
//od maxSize - 2 do length - 2
//czy Ty wiesz ze string ma metode substr????????????????
std::string excludeFromOneBehindLast(const Name & name)
{
	std::string destination;
	int amountLeavingCharactersWithoutLast = name.sizeMaximum - 1;

	for (int i = 0; amountLeavingCharactersWithoutLast > 0; i++)
	{
		destination += name.name[i];
		amountLeavingCharactersWithoutLast--;
	}
	destination += name.name.back();
	//i cala ta zabawe mozna uproscic do jednej linijki!
	return name.name.substr(0, name.sizeMaximum - 1) + name.name.back();
	return destination;
}

//bez przesady, kazdy rozumie co to jest cout :D
//to by moze mialo sens gdybys pisal obiektowo
void show(const Name & name)
{
	std::cout << name.name << std::endl;
}

int getTestsResult()
{
	//brzydki sposob sygnalizowania co poszlo nie tak
	//jak juz testujesz w ten sposob to uzyj jakiegos enuma
	if (!testNameSizeLowerThenMaximumSize())
		return 1;
	if (!testNameSizeEquelMaximumSize())
		return 2;
	if (!testTooManyAllOthers())
		return 3;
	if (!testTooManyPartOthers())
		return 4;
	if (!testTooManyAllDigits())
		return 5;
	if (!testTooManyPartDigits())
		return 6;
	if (!testTooManyAllVowels())
		return 7;
	if (!testTooManyPartVowels())
		return 8;
	if (!testTooManyAllWithoutVowel())
		return 9;
	if (!testTooManyAll())
		return 10;
	return 0;
}

bool testNameSizeLowerThenMaximumSize()
{
	Name name{ "AB$ 1", 6 };
	std::string expected = "AB$ 1";

	name.name = shortenToSizeMaximum(name);

	return name.name == expected;
}

bool testNameSizeEquelMaximumSize()
{
	Name name{ "AB$ 1", 5 };
	std::string expected = "AB$ 1";

	name.name = shortenToSizeMaximum(name);

	return name.name == expected;
}

bool testTooManyAllOthers()
{
	Name name{ "A_ B$1", 4 };
	std::string expected = "AB$1";

	name.name = shortenToSizeMaximum(name);

	return name.name == expected;
}

bool testTooManyPartOthers()
{
	Name name{ "A_ __ _B$1", 6 };
	std::string expected = "A_ B$1";

	name.name = shortenToSizeMaximum(name);

	return name.name == expected;
}

bool testTooManyAllDigits()
{
	Name name{ "A1B$123456789", 3 };
	std::string expected = "AB$";

	name.name = shortenToSizeMaximum(name);

	return name.name == expected;
}

bool testTooManyPartDigits()
{
	Name name{ "3A123B456$789", 4 };
	std::string expected = "3AB$";

	name.name = shortenToSizeMaximum(name);

	return name.name == expected;
}

bool testTooManyAllVowels()
{
	Name name{ "AAEIOUY$aeioDuy", 3 };
	std::string expected = "A$D";

	name.name = shortenToSizeMaximum(name);

	return name.name == expected;
}

bool testTooManyPartVowels()
{
	Name name{ "A$aeiouyBAEIOUY", 5 };
	std::string expected = "A$BUY";

	name.name = shortenToSizeMaximum(name);

	return name.name == expected;
}

bool testTooManyAllWithoutVowel()
{
	Name name{ "B$BE", 3 };
	std::string expected = "B$E";

	name.name = shortenToSizeMaximum(name);

	return name.name == expected;
}

bool testTooManyAll()
{
	Name name{ "B$ACB", 4 };
	std::string expected = "B$AB";

	name.name = shortenToSizeMaximum(name);

	return name.name == expected;
}

komentarz 23 czerwca 2018 przez MsMaciek123 Pasjonat (24,760 p.)

Dla ciebie @RafalS

Duży minus za nie używanie /* */ tylko // :P

#include "stdafx.h"
 
#include <iostream>
#include <algorithm>
#include <string>
 
/*duzy minus za wrzucenie wszystkiego w jeden plik
 
plus za polaczenie imienia z maksimum w strukturze
minus za napisanie tego proceduralnie
czyli plus za polaczenie danych razem
minus za odsperowanie zachowania od danych
funkcje sa odzielone od danych na których operuja
w podejsciu obiektowym wiekszosc z tych funkcji byłaby
metodami klasy i nie przesyłałbyś do każdej funkcji
const Name& name
 
nie czepialbym sie gdybys pisal w C, ale piszesz w C++
wiec pisz obiektowo :P
 
za bardzo ogolna nazwa struktury
gdyby ktos chcial wykorzystac Twoj kod jako biblioteke
w swoim kodzie to nazwa typu Name posrod calej reszty
struktur z ktorych by korzystal bylaby niejasne
tutaj nie jest, bo jest jedyna struktura*/
struct Name
{
    std::string name;
    /*ja bym zamienil - maxSize
    czesciej sie z takimi nazwami spotykam :P*/
    int sizeMaximum = 0;
};
 
/*duzy plus za pisanie testow
oraz duzy minus za umieszczenie ich w jednym
pliku z cala reszta i nie uzywanie frameworkow
do testowania
 
a teraz napisze czego spodziewam sie po tych nazwach
a potem zweryfikujemy czy nazwy byly dobrze dobrane
 
get sugeruje ze funkcja nie robi nic poza zwroceniem
test result, a prawdopodobnie puszcza ona wszystkie testy
druga sprawa co to jest testResult?
liczba testow ktore przeszly? 1 lub 0?
dla mnie to jest niejasne
generalnie takich funkcji sie nie tworzy bo testy pisze
sie we frameworkach*/
int getTestsResult();
/*literowka w Then :P
ale generalnie bardzo dobra nazwa
dobrze opisuje testowany przypadek*/
bool testNameSizeLowerThenMaximumSize();
//literowka w Equel
bool testNameSizeEquelMaximumSize();
/*totalnie nie mam pojecia co to moze testowac
za duzo wszystkich innych, co to za przypadek testowy?*/
bool testTooManyAllOthers();
//to samo
bool testTooManyPartOthers();
bool testTooManyAllDigits();
//part digits? czyli ze troche liter troche czegokolwiek?
bool testTooManyPartDigits();
bool testTooManyAllVowels();
bool testTooManyPartVowels();
bool testTooManyAllWithoutVowel();
bool testTooManyAll();
/*jak przeczytalem wszystkie to juz czaje co to bedzie
testowac, trzymajac sie Twojej konwencji ciezko to nazwac
ale np testNameTooLongWithDigitsOnly
testNameTooLongWithSomeDigits
a ogolne testTooManyAll po prostu
testNameTooLong, skoro moze zawierac cokolwiek to
nie ma zadnego ograniczenia wiec nie jest to 
specyficzny tylko normalny przypadek :P
 
 
nie mamy klasy shortener, wiec czemu robimy jej setup
zielonego pojecia co to moze robic
zwraca Name, ale jak tworzy to name, na jakiej podstawie?
skad bierze dane (pewnie cinem :D, ale trzeba to zaznaczyć)*/
...

komentarz 23 czerwca 2018 przez RafalS VIP (122,820 p.)

Wybacz ctr+k, ctrl+c :P. Myślałem, że nie ma skrótu na komentarze /**/, a nie chciało mi sie wklepywać. Dopiero teraz załapałem, że jak zaznaczę linijkę nie od początku to wstawia te lepsze :D

komentarz 24 czerwca 2018 przez RafalS VIP (122,820 p.)

Troche się z tym jeszcze pobawiłem i ten test:

bool testTooManyPartDigits()
{
	Name name{ "3A123B456$789", 4 };
	std::string expected = "3AB$";

	name.name = shortenToSizeMaximum(name);

	return name.name == expected;
}

jest skopany, zgodnie z założeniami zanim zaczniemy usuwać cyfry wszystkie $ i _ muszą być już wywalone. Czyli rozwiązanie jest złe :P

Zwracam trochę honoru. Faktycznie nie da się tego napisać w dosłowny sposób. A przynajmniej nie ponosząc kosztów usuwania elementów ze środka stringa, które notabene nie są aż takie duże, kod byłby o wiele czytelniejszy, pytanie na czym nam bardziej zależy - optymalizacji czy czytelności.

Aczkolwiek gdyby tylko robić tak:

std::string excludeUnnecessaryDigits(const Name & name)
{
	std::string destination;
	int charactersToRemove = name.name.length() - name.sizeMaximum;

	for (int i = name.name.length() - 1; i >= 0 ; --i)
	{
		if (!isdigit(name.name[i]) || charactersToRemove <= 0)
			destination += name.name[i];
		else
			charactersToRemove--;
	}
	//zwrocenie odwroconego stringa
	return std::string(destination.rbegin(),destination.rend());
}

to uniknęlibyśmy całego liczenia znaków które muszą zostać. W każdej z tych funkcji liczymy ile jakichkolwiek znaków musimy jeszcze usunąć, żeby nazwa była wystarczająco krótka. Lecimy od końca i jeśli mamy niecyfre to przepisujemy, jesli mamy cyfre i dalej musimy usuwać to usuwamy. Na koniec odwracamy string, bo tworzyliśmy go na opak, żeby nie dodawać elementów na początek stringa co jest bardzo nieoptymalne (wymaga przestawiania wszystkich innych elementów przy każdym insercie)

komentarz 24 czerwca 2018 przez RafalS VIP (122,820 p.)
edycja 24 czerwca 2018 przez RafalS

A tak by to wyglądało w co prawda nieoptymalnej wersji, ale bardzo czytelnej przepisując dosłownie sformułowany algorytm:

void excludeUnnecessaryDigits(Name & name)
{
    for (int i = name.name.length() - 1; i >=0 && name.name.length() > name.sizeMaximum; --i)
        if(isdigit(name.name[i]))
            name.name.erase(i);
}

Wtedy cały program składałby się z 4 takich funkcji i jednej sprawdzającej czy nie ma niedozwolonych znaków w nazwie, czego notabene Ty też nie robisz :P

Zwróć tylko uwage na to jakie to jest czytelne :D. Dopóki nazwa za długa usuwaj cyfry od końca.

odpowiedź 23 czerwca 2018 przez rafal.budzis Szeryf (85,260 p.)

Nie analizowałem całego ale w oczy rzuciła mi się ta funkcja. I tylko z nią będzie wiązała się moja ocena.

std::string shortenToSizeMaximum(Name name)//transfer by value give a transformation look
{
    if (isSizeMaximumExceeded(name))
    {
        name.name = excludeUnnecessaryOthers(name);
        if (isSizeMaximumExceeded(name))
        {
            name.name = excludeUnnecessaryDigits(name);
            if (isSizeMaximumExceeded(name))
            {
                name.name = excludeUnnecessaryVowelsStartFromSecoundVowel(name);
                if (isSizeMaximumExceeded(name))
                    name.name = excludeFromOneBehindLast(name);
            }
        }
    }
    return name.name;
}

Funkcja ma bardzo dużo zagnieżdżonych ifów zawsze staraj się unikać tego typu kodów często trudniej jest zrozumieć program gdy mamy tego typu ify szczególnie gdy w kazdym ifie bedzie kilka linijek kodu a ty sam nie bedziesz w stanie zrobaczyć wszystkich ścieżek ifów na jednym ekranie. Wystarczy odwrócić warunki i będziemy mieli :

std::string shortenToSizeMaximum(Name name)//transfer by value give a transformation look
{
    if (!isSizeMaximumExceeded(name)) {
        
        return name.name;
    }
    name.name = excludeUnnecessaryOthers(name);
    if (!isSizeMaximumExceeded(name)) {

        return name.name;
    }
    name.name = excludeUnnecessaryDigits(name);
    if (!isSizeMaximumExceeded(name)) {

        return name.name;
    }
    name.name = excludeUnnecessaryVowelsStartFromSecoundVowel(name);
    if (!isSizeMaximumExceeded(name)) {

        return name.name;
    }
    name.name = excludeFromOneBehindLast(name);

    return name.name;
}

Lepszym rozwiazaniem jest stworzenie takiego kodu który co prawda bedzie wykonywał wiecej porównań lecz jest bardziej czytelny i łatwiejszy w utrzymaniu.

std::string shortenToSizeMaximum(Name name)//transfer by value give a transformation look
{

    if (isSizeMaximumExceeded(name)) {
        name.name = excludeUnnecessaryOthers(name);
    }
    if (isSizeMaximumExceeded(name)) {
        name.name = excludeUnnecessaryDigits(name);
    }
    if (isSizeMaximumExceeded(name)) {
        name.name = excludeUnnecessaryVowelsStartFromSecoundVowel(name);
    }
    if (isSizeMaximumExceeded(name)) {
        name.name = excludeFromOneBehindLast(name);
    }
    
    return name.name;
}

Ale i tak naj naj lepszym rozwiązaniem było by stworzenie tych warunków isSizeMaximumExceeded w środku kazdej funkcji. Dzieki czemu niezależnie gdzie wywołasz funkcje np excludeUnnecessaryVowelsStartFromSecoundVowel to zawsze się poprawnie wykona lub zwróci dokładnie to co przekazałeś. Dzieki temu ze ify bedą w funkcja nasza funkcja shortenToSizeMaximum skróci się do postaci:

std::string shortenToSizeMaximum(Name name)//transfer by value give a transformation look
{
    name.name = excludeUnnecessaryOthers(name);
    name.name = excludeUnnecessaryDigits(name);
    name.name = excludeUnnecessaryVowelsStartFromSecoundVowel(name);
    name.name = excludeFromOneBehindLast(name);

    return name.name;
}

Myślę ze jeśli nie czytałeś z czystym sumieniem mogę ci polecić książkę Czysty kod napisaną przez Pana Robert C. Martin.