Hwork3 by medvedevvu · Pull Request #3 · medvedevvu/golessons

medvedevvu · 2019-12-24T15:03:03Z

No description provided.

JekaMas

Код, насколько я вижу, должен работать, но есть несколько замечаний. Большей частью они связаны с нейдачным выбором структур данных.

JekaMas · 2019-12-26T10:29:14Z

countshopprice/countshopprice.go

+)
+
+/* Добавление нового товра в справочник - если есть измениться цена  */
+func addItemsPrice(itemsPrice map[int]*mu.ItemPrice, item *mu.ItemPrice) string {


Проясни, зачем item *mu.ItemPrice по указателю нужно тут.

Этот вопрос сохранился и в новом коде

JekaMas · 2019-12-26T10:36:54Z

countshopprice/countshopprice.go

+func addItemsPrice(itemsPrice map[int]*mu.ItemPrice, item *mu.ItemPrice) string {
+	/* проверим наличие в каталоге */
+	fnd := false
+	vIdx := 0


Не понял идею itemsPrice map[int]*mu.ItemPrice. Зачем map[int]*mu.ItemPrice, ключами у тебя int, которые порядковые индексы. Но это усложняет поиск по map и сводит ее к slice []int. Почему бы не сделать map[string]*mu.ItemPrice, где string - это название товара? Тогда циклы с поиском исчезнут полностью.

JekaMas · 2019-12-26T10:37:44Z

countshopprice/countshopprice.go

+	if fnd {
+		msg += "--обновиление--цены--старая:" + fmt.Sprintf("%.2f", oldPrice) +
+			" новая:" + fmt.Sprintf("%.2f", item.ItemPrice)
+		itemsPrice[vIdx] = item


Несколько неудачное название itemsPrice - оно говорит о том, что это цены товаров, а там товары целиком. Может попробовать items?

JekaMas · 2019-12-26T10:38:56Z

mutils/structsdef/structsdef.go

@@ -0,0 +1,19 @@
+package structsdef
+
+// ItemPrice - описание товара


type Item struct { Name string Price float32 }

Может так? Выразительность даже лучше.

JekaMas · 2019-12-26T10:39:31Z

mutils/structsdef/structsdef.go

+}
+
+// User - описание пользователя
+type User struct {


ype User struct { Name string // Имя пользователя Account float32 // остаток на счету }

JekaMas · 2019-12-26T10:43:20Z

countshopprice/countshopprice.go

+*/
+func getOrderCost(itemsList map[int]*mu.ItemPrice, shopList mu.Order) float32 {
+	var ordrCost float32 = 0
+	for _, shopName := range shopList.Items { // бегу по списку товаров в заказе


Этот вложенный цикл тоже упрастится, если поменять тип itemsList, как описал выше.

JekaMas · 2019-12-26T10:44:06Z

countshopprice/countshopprice.go

+сохраним список товаров с ценой во время запроса от пользователя
+*/
+
+func compStrArr(in1, in2 []string) bool {


Неясно, зачем эта функция. Комментарий, как мне кажется, не от неё.

JekaMas · 2019-12-26T10:45:37Z

countshopprice/countshopprice.go

+func orderRegister(acountList map[int]*mu.User, // список пользователей
+	ordersPrice *[]mu.Order, // списки товаров с ценами
+	itemsPrice map[int]*mu.ItemPrice, // справочник товаров
+	billList map[int]map[int]float32, // список счетов


по описанию неясно, что это за map и что у неё за ключи

JekaMas · 2019-12-26T10:46:18Z

countshopprice/countshopprice.go

+		return
+	}
+	// проверим кредитоспособность
+	if ostatok <= 0 {


float32 точно всегда будет проходить это сравнение корректно?

JekaMas · 2019-12-26T10:46:49Z

countshopprice/countshopprice.go

+	// добавить ветку просмотра
+
+	tmp := seveListwithCost(ordersPrice, itemsPrice, itemsList)
+	var saldo float32 = ostatok - tmp.TotalSum


saldo := ostatok - tmp.TotalSum

JekaMas · 2020-01-21T13:56:42Z

countshopprice/countshopprice.go

+	fnd := false
+	vIdx := 0
+	msg := item.ItemName
+	var oldPrice float32 = 0


В golang это var oldPrice float32
А скорее var oldPrice float

JekaMas · 2020-01-21T13:58:17Z

countshopprice/countshopprice.go

+	vIdx := 0
+	msg := item.ItemName
+	var oldPrice float32 = 0
+	for cidx, citem := range itemsPrice {


По прежнему не хватает items map[string]Item, если принимать такой и подобные параметры, то уйдут всй эти куски кода с поиском item, user

JekaMas · 2020-01-21T15:12:50Z

countshopprice/countshopprice.go

+			break
+		}
+	}
+	if fnd {


found немного лучше - более полные имена предпочтительнее.

Общее правило таково: чем больше время жизни переменной и шире ее контекст, тем более распространенное имя стоит сделать. Короткоживущая переменная одного цикла - подойдёт i, idx, k, v; переменная для использования в нескольких проверках, в среднем куске кода на десяток и более строк кода - лучше чтобы это было полное слово index, validator, config, item, price; сложное понятие или переменная в большом методе или тем более уровня пакета - несколько слов: lowestPrice, areItemsSorted, SortByItemName.

JekaMas · 2020-01-21T15:14:12Z

countshopprice/countshopprice.go

+	for _, shopName := range shopList.Items { // бегу по списку товаров в заказе
+		fond := false
+		for _, item := range itemsList { // бегу по списку товаров в каталоге
+			if shopName == item.ItemName {


От этого стоит избавиться полностью - от цикла с поиском.

Может стоит сделать новые методы у shop - getUser, getItem, которые возвращают пользователя и товар по соответствующим именам.

JekaMas · 2020-01-21T16:04:19Z

countshopprice/countshopprice.go

+}
+
+func seveListwithCost(
+	ordersPrice *[]mu.Order, // списки товаров с ценами


зачем тут слайсы по указателю? по названию фукнции не очень понятно, что она должна делать

JekaMas · 2020-01-21T16:06:21Z

countshopprice/countshopprice.go

+	var ostatok float32 = 0
+	//var totalCost float32 = 0
+	var vIxd int = -1
+	for idx, iUser := range acountList {


Этот цикл надо убрать, как и вспомогательные к нему переменные.

JekaMas · 2020-01-21T16:19:46Z

countshopprice/countshopprice.go

+func showAccount(acountList map[int]*mu.User, p int) {
+	switch p {
+	case 0:
+		{


Тут не нужны {}

Этим константам стоит дать имена и тип, чтобы было очевидно при использовании

type sortDirection uint8 const ( ByNameAsc sortDirection = iota ByNameDesc // и так далее )

JekaMas · 2020-01-21T16:20:11Z

countshopprice/countshopprice.go

+func (a ByAcc) Less(i, j int) bool { return a[i].Account < a[j].Account }
+func (a ByAcc) Swap(i, j int)      { a[i].Account, a[j].Account = a[j].Account, a[i].Account }
+
+func showAccount(acountList map[int]*mu.User, p int) {


Посмотри, какое более очевидное имя можно дать для p

JekaMas · 2020-01-21T16:37:40Z

countshopprice/countshopprice.go

+
+	ordersPrice := []mu.Order{} // список заказов с посчитанной общей ценой
+
+	fmt.Println(seveListwithCost(&ordersPrice, itemsPrice, mu.Order{[]string{"Хлеб", "Сосиски"}, 0}))


Посмотри, как происходит заказ и все операции с магазином. Они очень много заставляют пользователя делать, создавать корректные данные, передавать их куда надо. Лучше было бы, чтобы это было скрыто от пользователя кодом Shop. Посмотри на предложенный интерфейс магазина, возможно это подскажет направление.

JekaMas · 2020-01-21T16:38:56Z

mutils/structsdef/structsdef.go

+}
+
+// Order - описание заказа
+type Order struct {


Комментарий всё ещё в силе.

JekaMas · 2020-01-21T16:39:46Z

mutils/structsdef/structsdef.go

@@ -0,0 +1,19 @@
+package structsdef


названия пакетов лучше делать по задачам или типам, которые определены в пакете. то же и про файлы.
item.go shop.go в пакете shop будут более понятны (это просто пример, возможно ты найдешь лучшее устройство кода).

JekaMas · 2020-01-21T16:53:09Z

mutils/structsdef2/fileutls.go

+//SaveAcountList - схраним пользователей
+func SaveAcountList(vacountList map[string]*User) {
+	if len(vacountList) == 0 {
+		panic(collectionEmpty)


То же про паники

JekaMas · 2020-01-21T16:54:27Z

mutils/structsdef2/structsdef2.go

+
+// SetDiscount - устанавливаем скидку на заказ в зависимости от пользователя
+// типов товаров и заказов
+func (order *Order) SetDiscount(userName string,


Возможно я неверно понял, но это больше похоже на подсчёт скидки, чем на ее установку в order данных.

получилось путано - там есть комментарий
244 Воспользуемся SetDiscount - сделаем скидку
тут собственно скидка устанавливается

JekaMas · 2020-01-21T16:57:00Z

shopcomplect/shopcomplect.go

+		itemsPrice[itemName] = item
+		msg += "--новый--товар--по цене--:" + fmt.Sprintf("%.2f", item.ItemPrice)
+	} else { // нашли - обноавляем цену и тип
+		msg += "--обновиление--цены--старая:" + fmt.Sprintf("%.2f", vItemPrice.ItemPrice) +


Очень много кода в разных методах, которые нужны только для вывода сообщений. Их стоит полностью убрать и заменить на тесты функций и методов.

JekaMas · 2020-01-21T16:58:11Z

shopcomplect/shopcomplect.go

+		vitemsList, ok := itemsList[name]
+		if ok {
+			switch vitemsList.ItemType {
+			case 2:


Трудно понять, что тут происходит. попробуй добавить имена константам, выделить подобные куски кода в отдельные функции.

JekaMas · 2020-01-21T16:58:33Z

shopcomplect/shopcomplect.go

+	   если больше 1,2,...,4....> товаров - то вернуть список без пробника
+	*/
+	switch {
+	case (len(probnik) == 1 && len(tovar) == 2):


Внешние скодки () лишние

JekaMas · 2020-01-21T17:21:31Z

mutils/structsdef2/fileutls.go

+	if r := recover(); r != nil {
+		switch r {
+		case openFileError:
+			{


Лишние {}

если оператор один , то {} - можно опускать ?

Даже если их много в конструкции switch внутри каждого case фигурные скобки не нужны

mutils/structsdef2/fileutls.go

JekaMas · 2020-01-21T17:24:32Z

mutils/structsdef2/fileutls.go

+			}
+		}
+	} else {
+		f.Close()


может в начале функции сделать

defer func() { if f != nil { f.close() } }()

не знаю почему , но go ругалась - дескать не вижу f !

JekaMas · 2020-01-21T17:26:34Z

mutils/structsdef2/structsdef2.go

+import "fmt"
+
+// ItemPrice - описание товара в дальнейшем map[ItemName] *ItemPrice
+type ItemPrice struct {


По сути, это не ItemPrice, а Item, который включает в себя не только цену.
Префикс Item- для полей структуры лишний, он не добавляет смысла.

// 0 - обычный товар 1 - премиум товар 2 - пробник с нулевой ценой
лучше сделать новый тип и для него определить эти три константы и дать им имена.

полагаю это надо было сделать через iota

JekaMas · 2020-01-21T17:27:42Z

shopcomplect/shopcomplect.go

+
+func addItemsPrice(itemsPrice map[string]*mu.ItemPrice,
+	itemName string,
+	item *mu.ItemPrice) string {


зачем item *mu.ItemPrice, равно как и itemsPrice - по указателю?

ну я раньше задавал вопрос - у меня не получалось обновить значение в структуре
было так acountList *map[int]mu.User , и я не мог выполнить (*acountList)[vIxd].Account = 10.
Ты рекомендовал acountList map[int]*mu.User - чтоб можно было это сделать , и я ещё
попробовал и спросил https://play.golang.org/p/YhSH0hAHHiX - верно или нет.
Получив утвердительный ответ, я рассудил , что если в структуре надо что-то менять, то нужно ставить * - по этому map[string]*mu.ItemPrice - а там я как раз меняю цену.

JekaMas · 2020-01-21T17:28:53Z

shopcomplect/shopcomplect.go

+   вернуть заказ с посчитанной ценой
+*/
+func getOrderCost(itemsList map[string]*mu.ItemPrice, shopList mu.Order) float32 {
+	var ordrCost float32 = 0


инициализация нулевым значением не нужна

это остаточное - со временем буду опускать, просто до автоматизма отработано - если завел переменную - присвой значение.

JekaMas

к новому коду комментарии

medvedevvu added 7 commits December 20, 2019 17:42

--- init commit ----

dfcf4db

--задание 1--

375fd64

-- 1.1 задание --

073e452

-- 1.2 задание --

27059ec

--задание 1 --- 22.12.2019--

a00e553

-- 1. задание 23.12.2019 --

e2fd92d

-- 1 задание 25.12.2019 --

29825c3

JekaMas reviewed Dec 26, 2019

View reviewed changes

medvedevvu added 13 commits December 26, 2019 19:52

-- 1 задание --- сортирует --

0ee79fe

-- 1 задание - теперь сортировка не слетает --

4289cd3

-- часть 2 - задания 1,2,3 --

5e82628

-- работа над ошибками --

0d1ac1c

-- изменил пользователей и товары --

ee2a810

-- Добавил скидочную функцию --

daa6e28

-- Добавил скидочную функцию 2 --

123e09b

-- добавил механизм скидки в алгоритм списания с аккаунта --

3c84800

-- добавил сохранение пользователей, счетов, каталога --

db07c2e

-- дописал сохранение данных в файл --

30ef2ed

-- доделал выгрузку/загрузку в файлы --

9ad4f79

-- чуть подправил , дописал README.md --

5d4fb68

-- удалил лишние файлы --

92caf65

JekaMas reviewed Jan 21, 2020

View reviewed changes

mutils/structsdef2/fileutls.go Show resolved Hide resolved

JekaMas reviewed Jan 21, 2020

View reviewed changes

		@@ -0,0 +1,19 @@
		package structsdef

		// ItemPrice - описание товара


		ordersPrice := []mu.Order{} // список заказов с посчитанной общей ценой

		fmt.Println(seveListwithCost(&ordersPrice, itemsPrice, mu.Order{[]string{"Хлеб", "Сосиски"}, 0}))

Comments

Conversation

medvedevvu commented Dec 24, 2019

Uh oh!

JekaMas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JekaMas Jan 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

JekaMas Jan 21, 2020 •

edited

Loading

JekaMas Jan 21, 2020 •

edited

Loading

medvedevvu Jan 21, 2020 •

edited

Loading