traverso-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Traverso-devel] ”audio import option“ early patch


From: Remon | Traverso-DAW
Subject: Re: [Traverso-devel] ”audio import option“ early patch
Date: Sun, 2 Jun 2019 12:44:51 +0200

Thanks for the patch, here some first feedback

: if (m_importDialog->exec() == QDialog::Accepted)

This will block the main event loop, according to Qt doc a better design would be to use show() or popup() if I remember correctly. 
This might influence the design, if you're not sure how to make this change, keep the current design and we'll be able to fix that later


: private_setEnableOrNot()

Traverso uses_function_names_like_this andNotLikeThis (although I made this mistake myself in TShortCutManager hehe). 
The reason is simple, Qt uses camelcase so it is easy to distinguish Traverso functions vs Qt functions

Also, private functions do not have a private_ nomenclature  in the function names. There is one particular case we do use that in Traverso and that is when adding/removing TAudioProcessingNodes to/from lists in the audio processing path, so in order to avoid confusion we just use identical function names nomenclature for private/protected/public functions

: TImportDialog
+void TImportDialog::private_numberOfFiles(int num)
+{
+
+    emit numberOfFiles(num);
+}

Seems like a design issue to call a function for the only purpose to emit a signal ?

: including headers
As far as I know including header files in header files can significantly increase compile time, better to avoid including header files in a header file as much as possible.

e.g. in TImportDialog.h I think that 
#include "AudioTrack.h" can be changed to class AudioTrack; and move the #include "AudioTrack.h"  to TImportDialog.cpp


In general it looks fine, similar code layout as anywhere else in Traverso, nice, keep it going!


Remon


Op za 1 jun. 2019 om 18:49 schreef 石任之 <address@hidden>:
I'm sorry for my delay.

In this patch:
Can only import the audio via menu now, no drag&drop.
Only the "Put Clips on their own track + Copy Audio to Project Directory” way work now;
_______________________________________________
Traverso-devel mailing list
address@hidden
https://lists.nongnu.org/mailman/listinfo/traverso-devel

reply via email to

[Prev in Thread] Current Thread [Next in Thread]