LINUX.ORG.RU
Форум — Development  

Поругайте код, плз

 , , поругайте


0

1

Небольшой кусок из отрисовки гуя програмым на Tk. Сижу переосмысляю архитектуру, реализацию. Перерабатываю 1500 строк, написанных как приведено ниже. Поругайте, подскажите как лучше.Заранее благодарю:

#Require main packages for work
package require Tk
# Data for non user settings
dict set nonUserSettings 0 title "tkProgram"
dict set nonUserSettings 0 minWidth 800
dict set nonUserSettings 0 minHeight 600
dict set nonUserSettings 0 resizableX 1
dict set nonUserSettings 0 resizableY 1
dict set nonUserSettings 0 minWidth 800
dict set nonUserSettings 0 minHeight 600
# Data for user settings
dict set userSettings 0 menuBar true 
# Data for biuld menubar
dict set menuBarSettings 0 name mainMenuBar
dict set menuBarSettings 0 tearOff 1
# Data for 0-level menu
dict set menuBarItems_ZeroLevel 0 name connectionItem
dict set menuBarItems_ZeroLevel 0 text_item "Connection"
dict set menuBarItems_ZeroLevel 1 name helpItem
dict set menuBarItems_ZeroLevel 1 text_item "Help"
# Data for 1-level menu
dict set menuBarItems_FirstLevel 0 parent 0
dict set menuBarItems_FirstLevel 0 label "LogIn"
dict set menuBarItems_FirstLevel 0 command NONE
dict set menuBarItems_FirstLevel 1 parent 0
dict set menuBarItems_FirstLevel 1 label "LogOut"
dict set menuBarItems_FirstLevel 1 command NONE
dict set menuBarItems_FirstLevel 2 parent 0
dict set menuBarItems_FirstLevel 2 label "Exit"
dict set menuBarItems_FirstLevel 2 command { exit; }
dict set menuBarItems_FirstLevel 3 parent 1
dict set menuBarItems_FirstLevel 3 label "About"
dict set menuBarItems_FirstLevel 3 command NONE
dict set menuBarItems_FirstLevel 4 parent 1 
dict set menuBarItems_FirstLevel 4 label "Help"
dict set menuBarItems_FirstLevel 4 command NONE
# Procedure for builing menu bar
proc assembleMyMenu {} {
    global userSettings
    global menuBarSettings
    global menuBarItems_ZeroLevel
    global menuBarItems_FirstLevel
    if {[dict get $userSettings 0 "menuBar"]==true} {
        menu .[dict get $menuBarSettings 0 "name"]
        . config -menu .[dict get $menuBarSettings 0 "name"]
        dict for {id info} $menuBarItems_ZeroLevel {
            dict with info {
                menu .[dict get $menuBarSettings 0 "name"].$name
                .[dict get $menuBarSettings 0 "name"] add cascade -label $text_item -menu .[dict get $menuBarSettings 0 "name"].$name
                dict for {iter data} $menuBarItems_FirstLevel {
                    dict with data {
                        if {$id==$parent} {
                            .[dict get $menuBarSettings 0 "name"].$name add command -label $label -command $command
                        }
                    }
                }
            }
        } 
    }
}

# Window Manager main settings
wm title . [dict get $nonUserSettings 0 "title"]
wm resizable . [dict get $nonUserSettings 0 "resizableX"] [dict get $nonUserSettings 0 "resizableY"]
wm minsize . [dict get $nonUserSettings 0 "minWidth"] [dict get $nonUserSettings 0 "minHeight"]
option add *tearOff [dict get $menuBarSettings 0 "tearOff"]
assembleMyMenu

в zip c паролем и никто не увидит

anonymous
()
Ответ на: комментарий от iMushroom

он просто высокомерный мудак, тикль тут не при чем. Ладно, это уже личное :)

nokachi ★
()

А у вас это что, в одном файле? Вы эти dict setы запилите в отдельном файле, а то смотрится как стена текста. Еще, лучше бы не делать dict set руками, а то много подробностей торчит наружу. Сделайте для этого процедуру которая берет параметры: словарь, parent, label и т.д. и делает dict set . Ну и наконец глобальные переменные во все поля - ай-ай-ай, нехорошо!

no-such-file ★★★★★
()

Как минимум отдели настройки от кода. Потом вот эти вот номера меню это очень плохо. 

loadLocal "ar" $LANGUAGE_ID; #  loadLocal придумай сам

dict set menuBarItems_FirstLevel login parent 0
dict set menuBarItems_FirstLevel login label $ar(login); # эта строка должна вообще функцией загружаться отдельной как минимум
dict set menuBarItems_FirstLevel login command NONE
Ну я бы типа такого делал, если бы вообще так делал.

Suntechnic ★★★★★
()
Последнее исправление: Suntechnic (всего исправлений: 1)
Ответ на: комментарий от Suntechnic

Ну и вообще dict это модно но лучше чтобы он наружу не торчал. Юзай списки или массивы - они быстрее, чтобы не говорили гуру.

Я выношу все настройки в файлы вида: 

DB_TYPE redis
DB_NAME 8
DB_LOGIN ""
DB_PASS ""
Пользователю легко писать, в коде элементарно загрузить в список или массив.

Suntechnic ★★★★★
()

Я не знаю тикля, но на Tkineter (Python) сделал приблуду, которая на основе древовидной структуры данных (имя ноды, ф-я или подменю) делала меню. С остальными виджетами аналогично. А так это вырвиглазие X(

AIv ★★★★★
()
Ответ на: комментарий от no-such-file

Пока в одном. Разносить на разные файлы еще не начинал :). Про глобальные переменные - спасибо. Про словарь - спасибо, вы правы. Пометку сделал

iMushroom
() автор топика
Ответ на: комментарий от nokachi

Ахах, не вижу ничего плохого в данном коде). Ну а «так», к сожалению, только тиклю позволяет писать

iMushroom
() автор топика

Длинные строки, ИМХО лучше переносить с использованием бэкслеша.

olibjerd ★★★★★
()

это ужастно.

любой стиль хорош, когда он легко читаем. Ваши декларации нечитаемы вообще.

придумайте свой 'язык' (структуру) для описания интерфейса и юзайте её.

код за 10 мин, то-же (даже больше) но читается проще: 

package require Tk
package require msgcat
# menu structure
set mainmenu {
	cascade "connection" {
		command "login" {
		}
		command "logout" {
		}
		command "exit" {
		    destroy .
		    # wrong, should by "event generate <<AppExit>>"
		}
	}
	cascade "help" {
		command "about" {
		}
		command "help" {
		}
	}
}
# lang. translations
array set en {
    "help" "help please"
    "exit" "dismiss"
}
proc tr { word {langname en} } {
    upvar #0 $langname lang
    set word [ string tolower [ string trim $word ] ]
    if [ info exists lang($word) ] {
	set lang($word)
    } else {
	msgcat::mc $word
    }
} 
proc Tr { word {langname en} } {
    string totitle [ tr $word $langname ]
}
# menu 'builder'
proc factory { w decl {top {}} } {
	set m [ menu $w -tearoff 0 ]
	set subid 0
	foreach { type id options } $decl {
		switch -exact -- $type {
			"command" {
				if { $options == {} } {
				    # setup signals
				}
				$m add command -label [Tr $id] -command $options
			}
			"cascade" {
				set sub [ factory $m.sub$subid $options ]
				$m add cascade -label [Tr $id] -menu $sub
				incr subid
			}
			default {
				puts stderr "unknown type in decls $type"
			}
		}
	}
	set m
}
# generate menu from declaration
factory .mainmenu $mainmenu
# bind signals
# attach menu
. configure -menu .mainmenu
tkwait window .

кстати удобные виджеты в snit и itcl

MKuznetsov ★★★★★
()

как уже сказали :

излишне буквально.

прикрути что ли предметную область.

qulinxao ★★☆
()

блин, это ж так на tcl написать... первый раз вижу

foreach {x y z} {
                0 title tkProgram
                0 minWidth 800
                0 minHeight 600
                0 resizableX 1
                0 resizableY 1
                0 minWidth 800
                0 minHeight 600
                } {
 dict set nonUserSettings $x $y $z
 }

начать хотя бы с этого, что ли

anonymous
()
Ответ на: комментарий от anonymous

я бы сделал что-нибудь вроде модуля mymenu.tcl, туда бы вписал

package require Tk

namespace eval MyMenu {
 var nonUserSettings
 ...

 proc Assemble {} {
  # сборка меню
  ...
  }

 proc Init {} {
  # инициализация модуля
  ...
  }

 Init
 }

а в вызывающем коде:

source mymeny.tcl MyMenu::Assemble

dict скорее лишнее, даже в буквальном переводе работа с обычным массивом set nonUserSettings(0,title) «tkProgram» будет проще, быстрее, короче и понятнее ИМХО

anonymous
()
Ответ на: комментарий от anonymous

склеилось 

source mymeny.tcl
MyMenu::Assemble

anonymous
()
Вы не можете добавлять комментарии в эту тему. Тема перемещена в архив.
Development