delthas / JavaSkype

A lightweight, comprehensive Java API for Skype using MSNP24
MIT License
72 stars 28 forks source link

[ArchLinux] No live:account login, not showing last messages #38

Closed mvasi90 closed 7 years ago

mvasi90 commented 7 years ago

This is very very bad written in Java. Maven is not necessary:

First: You are using the Java Native xml Document with Jsoup parsed Document. What is it?

import org.w3c.dom.Document;
import org.w3c.dom.Node;
import org.w3c.dom.NodeList;
import org.xml.sax.InputSource;
import org.xml.sax.SAXException;

If you are using Jsoup this is not necessary (or if you are using regular expressions).

I have no time to modify the code and push in separate branch to github. Because full rewrite is needed.

Bad performance. What is it?

NodeList conversationNodes = doc.getElementsByTagName("conversation");
List<String> threadIds = new ArrayList<>();
outer:
for (int i = 0; i < conversationNodes.getLength(); i++) {
   Node conversation = conversationNodes.item(i);
   String id = null;
   boolean isThread = false;
   NodeList conversationChildren = conversation.getChildNodes();
...
...

You are using "branching statements". outer:. Please don't do this, is very bad practice.

You should use only the Jsoup library or only regular expressions. (You are using Jsoup parser with org.w3c.dom.Document) Example:

Elements conversations = body.select("conversation");
 for (Element conversation : conversations) {
// this is not necessarry at the moment because the messageid contains the id
//  String id = conversation.getElementsByTag("id").first().text();
    Elements messages = conversation.getElementsByTag("message");
    for (Element message : messages) {
    String originalarrivaltime = message.getElementsByTag("originalarrivaltime").first().text();
    String composeTime = message.getElementsByTag("composetime").first().text();
    String from = message.getElementsByTag("from").first().text();
    String messageType = message.getElementsByTag("messagetype").first().text();
    String content = message.getElementsByTag("content").first().text();
    String conversationID = message.getElementsByTag("conversationid").first().text();
    User u = (User) parseEntity(from);
    try {
       skype.userMessageReceived(conversationID, u, dateFormatA.format(dateFormatDe.parse(originalarrivaltime)), dateFormatA.format(dateFormatDe.parse(composeTime)), messageType, content);
       } catch (java.text.ParseException ex) {
          ex.printStackTrace();
                  }
          }
}

The above code works to show the last offline/online messages of contacts. Retouching that kind of code leaves me with rage. I do not know you and I do not know who taught you to program but you can tell him that he is a bad teacher.

You have a simple problem and you are creating "sophisticated" and wrong solutions. In other words, you are catching the server response but you don't show it to the user. Instead of showing it you are creating incomplete filters that are using more resources. Remember: Jsoup not needed. (And if you have more time, Json is not needed)

Maven? Never use it. If you want to have automated things use Gradle (best performance) https://gradle.org/maven-vs-gradle/

The README file show an example to use this library. Before invoking the connect() method, the listeners must be registered.

PostData: I never use Gradle, Maven, etc.

delthas commented 7 years ago

The Github issue tracker is designed to leave suggestions, bug reports, or general questions on a project. This issue is only a rant about my coding style. As it is highly irrelevant, I am closing and locking this issue.

I do not want to discuss the various matters you have written about in your issue, because:

If you have a specific suggestion, bug report, or general question, please open another issue and send me debug logs if needed. You may take a look at the other open issues on my repository as examples to understand how and whether to open an issue.

delthas commented 7 years ago

Additionally, I will not accept any pull request whose only purpose is to change the project code style. Thanks for your understanding.