Podcast #128: We chat with Kent C Dodds about why he loves React and discuss what life was like in the dark days before Git. Listen now.

Hot answers tagged

30

Checking TCP ports and multi-threading Your performance should be fine for a sequential program. Another way to check whether the TCP port is open is by just sending the initial SYN and wait for the ACK. That way, you don't have to do the complete TCP handshake. However, that also means that you have to implement that part of the TCP protocol yourself. Also,...


18

The second one is much easier to read, it expresses the developers intent. At first sight I (as a maintainer or another developer in the same team, for example) just want a quick overview about the code and don't care about the details. (How the code creates a server socket, for example.) The second one exactly gives that. The System.exit side effect could ...


18

Design This is a very thin C like wrapper around sockets. In my opinion there are a lot of style changes that are need to make this good C++ or usable by modern C++ library. Main issues: Two phase construction Error Codes escape public interface (use exception) You member variables have state not related to the object after construction. Your use of ...


16

In your chatGUI's __init__ your setting up some constants, that should really just be defined as constants at the class level: class chatGUI: #Class That Handles GUI related tasks. BGCOLOUR = "#2a2a2a" # First Colour was: "#607D8B" # Contains hyperlink triggers. HYPER_LIST = ("http://", "www.", "https://", "ftp://") WINDOW_TITLE = "Oke's ...


15

I have a few comments that are unrelated to the synchronous/asynchronous and/or header-only nature of the code. parameters bools I don't like passing bools as parameters. I really dislike a function like your SimpleSocketStreamBuffer constructor that take multiple bools. You need to do a fair amount of looking to be sure how: foo x("www.google.com", ...


15

I can't resist saying this… the way you use a lot of whitespace to achieve your horizontal alignment, especially in server.cpp, is distracting and hinders readability. I don't mind it so much when several lines have some relationship to each other. For example: serverAddr.sin_family = AF_INET; serverAddr.sin_port = htons(8080); serverAddr....


14

My main concern is the use of eval() without proper validation (well, the regular expression extraction puts some limits onto the possible expression values, but still it can be exploitable). I would either switch to tools like numexpr (there are some other options at this topic), or, taking into account the simplicity of the possible expressions, map the ...


13

I will not comment on the actual TCP functionality itself. I am not competent for that. Stopping broadcast - on purpose? When you are broadcasting to the clients in SendMessage(), it seems like you stop broadcasting if you get problems reaching one of the clients. Is this intended? I would move the try/catch inside the foreach, and around the if. Then, in ...


13

Don't use wildcard imports Instead of this: from tkinter import * Do this: import tkinter as tk With the latter, you then need to prefix all of your tk commands and classes with tk. (eg: tk.Tk()). This is a tiny bit of extra typing, but makes your code more readable. At some point you may choose to use ttk widgets which defines many classes with the ...


12

Your exception handling for new ServerSocket(port) could be better. I don't like the way System.exit(1) is buried in a helper function. The exception should be caught in such a way that it bypasses the clientSocket-accepting loop. Similarly, if serverSocket.accept() throws an exception, you log it, but blithely proceed to try to create a client thread ...


12

Observations as I'm reading down the code: Server Class doesn't implement an interface. If I'm using your library, at least one of my classes is going to have a dependency on the Server class. If I use an IoC container, I need to pick one that can work with concrete types (that's usually not a problem though). If I want to write unit tests for that class ...


11

It's not Pythonic to pass in a mutable object and use that to store results (like it is in C). range(start, stop) is not inclusive of stop, so you have an off by one error as well. Using a catch-all except line is also poor practice. It's important to catch expected exceptions and reraise the rest. Also Python lists are thread safe only because of the GIL ...


11

I'm not excited about the structure of the code. I think you're putting more in one class than is really a good idea. Just for example, I'd start with a a tiny class that does nothing but handle the WSAStartup/WSACleanup sequence: class WSAUser { WSADATA data; public: WSAUser() { /* call WSAStartup() */ } ~WSAUser() { /* call WSACleanup() */ } ...


11

Pretty sure this is not portable. long length = htonl( i.length() ); ret = ::send( hSocket, reinterpret_cast<const char*>(&length), sizeof(length), 0 ); You are obviously trying to make it portable (good). But htonl() does not return a long which is platform/compiler dependent. What you want really want is uint32_t (man htonl). Anything ...


11

I think you have too many different responsibilities bound up into one class. That ends up not only making that class more complex, but adding extra complexity overall as well. As a starting point, I'd have a really trivial class that does nothing but stack-based management of the WSAStartup/WSACleanup: struct socket_user { socket_user() { ...


11

I don't see anything in this code that needs to be commented. To echo what other people said, comments should clarify code and they should never say what the code already says. I would recommend commenting code under these conditions: You can't tell what the code is doing by reading it You encountered a wonky bug that took you a long time to figure out, ...


11

Commenting the method At the minimum, public API methods should always be commented. The posted code is an implementation. You should have the method declaration, serving as the interface definition, somewhere else. It's best when the actual implementation is not even visible to users of your API. That's good information hiding. So, where this method is ...


11

A few notes: As Jamal noted in the comments, your definitions of STDIN and STDOUT are somewhat useless. You can see here that using the given STDIN_FILENO and STDOUT_FILENO are already given those values. To counter your claim in the comments that they help readability, I would argue that they actually clutter up your code and should be removed. Declaring ...


11

One of the issues with TCP sockets is that there's no guarantee that sending N bytes in one go will result in receiving exactly N bytes after one call to read(). I would thus emphasize this by using a separator in the messages (newline perhaps?) and only printing out the result of what I've read after the whole line has been received. I'd also try to get ...


10

I'd prefer to see more safety, more structured code, and more abstract symbols instead of magic values. There is some inefficiency due to repeated calls that should be cached. Safety You're connecting to the net. You don't set a limit on the size of your input data, and you don't set a limit on the number of times you will respond to the server. I think ...


10

There is no guarantee that write writes out the entire buffer. This means that if (FD_ISSET(STDIN_FILENO, &fds)) rv = transfer(STDIN_FILENO, buffer, BUFLEN, sd); may lose data. Consider looping until all data gone out. As long as the portability is concerned, the timeout parameter of select needs attention. The BSD implementation doesn't change it, ...


10

the posted code causes the compiler to output several warning messages. When compiling, always enable the warnings, then fix those warnings. Here is the compile statement and the resulting warning messages. gcc -Wall -Wextra -Wconversion -pedantic -std=gnu11 -ggdb -c "untitled2.c" untitled2.c: In function ‘input’: untitled2.c:86:37: warning: conversion ...


9

0: Keep class members private unless there is a darn good reason to expose them. And then, if you have to, use properties. 1: Use readonly on class declarations which are considered unmodifiable after construction. Example: public readonly ManualResetEvent connected = new ManualResetEvent(false); This declares intent and keeps other code from accidentally ...


9

WSAGetLastError() returns an int. Throwing an int is legal but not quite usual. Your catch(std::exception& e) statements are fairly useless, given that Win32 APIs are unlikely to throw a std::exception object. Your default constructor is dangerous: you can't tell if a socket instance has been properly-constructed or default-constructed. You should ...


9

There are a number of things in here which could be a problem, and a number of other things which are a problem.... Ephemerals First up, you may just be running in to resource limitations. A TCP based computer has just 64K ports available for sockets. The first 1K are reserved for root, and the remainder are available for programs. You may think you have > ...


9

Detect closed connection 1 select promises that a read wouldn't block. This is a case when there are data, but this is also a case that the remote end is closed. In this case read immediately returns 0. Without testing for len > 0 in transfer end up in an infinite empty select/read cycle. Detect closed connection 2 The remote may close connection while ...


9

Your code has some incorrect assumptions. an application may listen on a specific address/port combination; 127.0.0.1:port can be available while *:port is not. an application may bind a port without listening. Connects will fail, but so will your own bind. a firewall or other mechanism can interfere with connections, generating false positives in your ...


8

I can't comment on how secure your code is, however there are a couple of other problems that I would like to point out. Clutter Your using directives can be cut down to: using System; using System.Net; using System.Net.Sockets; using System.Net.Security; using System.Security.Cryptography.X509Certificates; Naming Let me show you what I think of when I ...


8

Not much to comment on, this program is pretty straightforward. A few notes: The user of the client has a lot of information to input. fprintf(stderr, "Usage: %s <Server IP> <Server Port> <Iterations> <Gap>\n", argv[0]); The more the user has to enter, the steeper the initial learning curve to use the program is. Also, I'm not ...


8

I don't like either solution (or the currently suggested answers). In any application where you run a ServerSocket, I believe that the socket management should be done on the Main thread. This would avoid the exact situation where you have a System.exit(1). The basic pattern for a server application is: public static void main(String[] args) { // sort ...


Only top voted, non community-wiki answers of a minimum length are eligible