A method to log and display text in a console screen simultaneously. I had always had the necessity to display long text outputs on the console screen. The size of the text displayed usually exceeds the amount of text that can be accessed by scrolling up through he console output. Also, there had been times when I wanted to look at the output history, to spot out abnormalities or bugs. Doing this was difficult until I decided to write a  logger. Logging all the output text to a file, and simultaneously displaying it solves the above problems.

Source code:

#ifndef FILELOGGER_H_
#define FILELOGGER_H_

#include <fstream>
#include <iostream>
#include <string>

class FileLogger;

template <class Type>
FileLogger& operator << ( FileLogger& logCout, const Type& data);

class FileLogger {
    std::string fileName;
    bool __linkWithFile;
    bool __linkWithCout;
    bool __appendToFile;
public:

    FileLogger(const std::string& pfileName, bool pAppendToFile = true): fileName(pfileName),
                                              __linkWithFile(true),
                                              __linkWithCout(true)
                                              {
        if (pfileName == "<null>") {
             __linkWithFile = false;
        }

        __appendToFile = pAppendToFile;
    }
    FileLogger(const FileLogger& flog) {
        fileName       = flog.fileName;
        __linkWithFile = flog.__linkWithFile;
        __linkWithCout = flog.__linkWithCout;
        __appendToFile = flog.__appendToFile;
    }

    void linkWithCout() {
        __linkWithCout = true;
    }

    void unlinkWithCout() {
        __linkWithCout = false;
    }

    void linkWithFile() {
        __linkWithFile = true;
    }

    void unlinkWithFile() {
        __linkWithFile = false;
    }

    template <class Type> friend FileLogger& operator << ( FileLogger& logCout, const Type& data);

    void flush_console() {
        std::cout.flush();
    }

    ~FileLogger(){}

};

template <class Type>
FileLogger& operator << ( FileLogger& logCout, const Type& data) {
    if(logCout.__linkWithCout)
        std::cout << data;
    if(logCout.__linkWithFile) {
        std::ofstream fout;
        fout.open(logCout.fileName.c_str(),
                                            ((logCout.__appendToFile) ?(std::ios::out | std::ios::app) :
                                                    std::ios::out) );
        logCout.__appendToFile = true;
        fout << data;
    }
    return (logCout);
}

#endif

Example usage:

FileLogger logCout ("log.text", false);

// writes to both the screen and to the file `log.text`.
logCout << "Hello " << "World";
// appends the next line to the file by default.
logCout << "!";

 Explanation:

The functions, void linkWithCout() and void unlinkWithCout() can be used to force the class to write the output text to the screen and refrain from writing the output text to the screen respectively.
The functions, void linkWithFile() and void unlinkWithFile() can be used to force the class to write the output text to the file and refrain from writing the output text to the file respectively.

Is there a better way to achieve this functionality? Or is there any possible modifications that could help this become more useful (probably like linking this to multiple log files)?

Is it necessary?

I don't see what this achieves that can't be done by piping the output of your program through tee or similar; it just removes flexibility (you can no longer not save the logs).

Naming

All identifiers containing double underscore are reserved for the implementation for any purpose - including as macros.  So you'll need to remove those from the relevant names.

Allow moving rvalues

In the constructor, it's better to accept the file name by value, and std::move() it into the member:

FileLogger(std::string pfileName, bool appendToFile = true)
    : fileName(std::move(pfileName)),
      linkWithFile(fileName!="<null>"),
      linkWithCout(true),
      appendToFile(appendToFile)
{}

Omit what can be defaulted

There's no need for a copy constructor that's identical to the compiler-provided one, so just omit that - and then you won't inhibit the generation of a move constructor.

Extensibility

How about a more composable, general-purpose tee_stream class?  Then you can output to as many locations as you like, chosen on the fly at runtime.

Don't surprise your users

Given FileLogger logCout ("log.text", false);, then  logCout << "Hello " << "World"; opens the file, writes Hello to it, closes it, then opens it again, truncating and replacing the contents with World.  Most users would expect both Hello and World to be present after that line.

You should document somewhere that users can't write to a file called <null> - or change the magic name to a better value, such as the empty string (then the test becomes !fileName.empty()).


Swapping alternate nodes of linkedlist package main

import (
    "fmt"
)

type LinkedList struct {
    first, last *node

}

type node struct {
    item int
    next *node

}

func (l *LinkedList) add (val int) {
    n := node {
        item: val,
        next: nil,
    }
    if l.first == nil {
        l.last = &n
        l.first = &n
    } else {
        l.last.next = &n
        l.last = &n
    }

}

func (l *LinkedList) traverse () {
    for n := l.first; n != nil; n = n.next {
        fmt.Printf("%v ", n.item)

    }

    fmt.Println()

}

func (l *LinkedList) swapLL () {
    if l.first == nil {
        panic("First element cannot be null")
        }

    if l.first.next == nil {
        return
    }

    ptr1 := l.first
    var ptr2 *node
    var ptr3 *node
    var prev *node

    for ; ptr1 != nil && ptr1.next != nil; {
        //Allocate resources
        ptr2 = ptr1.next
        ptr3 = ptr2.next

        //swap
        ptr2.next = ptr1
        ptr1.next = ptr3

        //hook to the previous pair
        if prev == nil {
            l.first = ptr2
        } else {
            prev.next = ptr2
        }

        //advance
        prev = ptr1
        ptr1 = ptr3
    }

}

func main () {
    l := LinkedList {
        first: nil,
        last: nil,
    }

    l.add(10)
    l.add(20)
    l.add(30)
    l.add(40)

    l.traverse()

    l.swapLL()

    l.traverse()
}

Input:

Linkedlist:  10 -> 20 -> 30 -> 40 -> null

Output:

Linkedlist: 20 -> 10 -> 40 -> 30 -> null

Question:

Here 10 and 20 are adjacent thus they were swapped. Likewise, 30 and 40 are adjacent thus they were swapped.

I am looking for suggestions for improving my code, any optimizations, or anything to make my code more readable and elegant.

Welcome!

You may be interested in viewing the Go API implementation of a linked list. Their implementation is not restricted to just integer node data.

Use go fmt

A few of your lines have trailing whitespace. Likewise, some of your formatting isn't standard for Go.

If you run go fmt on the source code, it cleans all that up for you.

Don't unnecessarily export things

Your type LinkedList is exported. If this is for a library, you should add a comment explaining it's usage (used to generate documentation: see Godoc). If it's not for a library, it shouldn't be exported.

Going forward, I'll assume it's not meant for a library. Otherwise the fields of LinkedList also need to be exported, and the node type as well.

Avoid extra typing when initializing a struct

When initializing a struct, you don't have to initialize the fields to values that would already be the default.

See §Composite Literals in the language specification for more details.

n := node{
    item: val,
    next: nil,
}

Can be written as:

n := node{
    item: val,
}

And

l := linkedList{
    first: nil,
    last:  nil,
}

Becomes

l := linkedList{}

Combine variable declarations:

We can combine multiple variable declarations under one var keyword.

var ptr2 *node
var ptr3 *node
var prev *node

Becomes either

var ptr2, ptr3, prev *node

Or

var (
    ptr2 *node
    ptr3 *node
    prev *node
)

I prefer the shorter notation most times when the variables are of the same type, but either is better.

Use the specific Printf verb

You use the %v verb, but here we know at compile time that we're printing integers. You can safely use %d instead.

Return an error instead of panic()ing

Unless you expect to always recover() from the panic(), you should return an error value instead. This is more common.

func (l *linkedList) swapLL() {
    if l.first == nil {
        panic("First element cannot be null")
    }

    if l.first.next == nil {
        return
    }

    //...
}

Becomes

func (l *linkedList) swapLL() error {
    if l.first == nil {
        return fmt.Errorf("List cannot be empty")
    }

    if l.first.next == nil {
        return nil
    }

    //...

    return nil
}

This now makes it easy to check for the error:

if err := l.swapLL(); err != nil {
    log.Fatal(err)
}

Conclusion

Here is the final source I ended up with.

package main

import (
    "fmt"
    "log"
)

type linkedList struct {
    first, last *node
}

type node struct {
    item int
    next *node
}

func (l *linkedList) add(val int) {
    n := node{
        item: val,
    }

    if l.first == nil {
        l.last = &n
        l.first = &n
    } else {
        l.last.next = &n
        l.last = &n
    }
}

func (l *linkedList) traverse() {
    for n := l.first; n != nil; n = n.next {
        fmt.Printf("%d ", n.item)
    }

    fmt.Println()
}

func (l *linkedList) swapLL() error {
    if l.first == nil {
        return fmt.Errorf("List cannot be empty")
    }

    if l.first.next == nil {
        return nil
    }

    ptr1 := l.first

    var ptr2, ptr3, prev *node

    for ptr1 != nil && ptr1.next != nil {
        //Allocate resources
        ptr2 = ptr1.next
        ptr3 = ptr2.next

        //swap
        ptr2.next = ptr1
        ptr1.next = ptr3

        //hook to the previous pair
        if prev == nil {
            l.first = ptr2
        } else {
            prev.next = ptr2
        }

        //advance
        prev = ptr1
        ptr1 = ptr3
    }

    return nil
}

func main() {
    l := linkedList{}

    l.add(10)
    l.add(20)
    l.add(30)
    l.add(40)

    l.traverse()

    if err := l.swapLL(); err != nil {
        log.Fatal(err)
    }

    l.traverse()
}

Hope this helps!


Converting numbers to English words This is a simple program to convert a number, from 0 up to 99999999, to English words.

Example:

If the input is:

1234

The output would be:

  one thousand two hundred thirty four

I would like to get feedback regarding this small program.

#include <iostream>
#include <vector>
#include <string>

const std::vector<std::string> words[] =
{
    {
        "",
        "one",
        "two",
        "three",
        "four",
        "five",
        "six",
        "seven",
        "eight",
        "nine",
        "ten",
        "eleven",
        "twelve",
        "thirteen",
        "fourteen",
        "fifteen",
        "sixteen",
        "seventeen",
        "eighteen",
        "nineteen"
    },
    {
         "-",
         "-",
         "twenty -",
         "thirty -",
         "forty -",
         "fifty -",
         "sixty -",
         "seventy -",
         "eighty -",
         "ninety -"
    },
    {
         "million",
         "hundred",
         "thousand",
         "hundred",
         ""
    },
};

class NumberConvertor
{
public:
    NumberConvertor(std::size_t number);

    friend std::ostream& operator<< (std::ostream& out, const NumberConvertor& nc);

private:
    void convert(std::size_t unit, const std::string& units);

    std::size_t mNumber;
    std::string mResult;
    std::size_t mIterator;
    std::size_t mSwap;
};

NumberConvertor::NumberConvertor(std::size_t number)
    : mNumber(number), mIterator(0), mSwap(0)
{
    if (mNumber == 0)
    {
        mResult = "zero";
    }
    else
    {
        convert( 1000000,  words[2][0]);
    }
}

void NumberConvertor::convert(std::size_t unit, const std::string& units)
{
    std::size_t index = mNumber / (unit * 10);
    std::size_t sub   = (index > 1 ? index * unit * 10 : 0);

    std::string word = words[1][index];

    if (word[word.size() - 1] == '-')
    {
        word.erase(word.size() - 1);
        index   = (mNumber-sub) / unit;
        word.append(words[0][index]);
    }

    if (word != "")
    {
        std::size_t x = mNumber / 1000;

        switch(x)
        {
            case 100:
            case 200:
            case 300:
            case 400:
            case 500:
            case 600:
            case 700:
            case 800:
            case 900:
            {
                mResult.append( word + ' ' + units + ' ' + words[2][mIterator + 1] + ' ');
                break;
            }
            default:
            {
                mResult.append( word + ' ' + units + ' ');
            }
        };
    }

    mNumber -= mNumber / unit * unit;

    if( ++mIterator < words[2].size())
    {
        mSwap ^= 1;

        if(unit == 1)
        {
            convert( 1, words[2][0]);
        }

        convert( unit / (mSwap == 0 ? 100 : 10), words[2][mIterator]);
    }
}

std::ostream& operator<< (std::ostream& out, const NumberConvertor& nc)
{
    return out << nc.mResult;
}

int readInput(const std::string& prompt)
{
    int answer;
    while((std::cout << prompt) && (!(std::cin >> answer) || answer < 0 || answer > 99999999))
    {
        std::cout << "Enter a positive number between 0 - 99999999\n";
        std::cin.clear();
        std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
    }
    return answer;
}

int main()
{
    while(1) // (ctrl c) to exit
    {
        std::cout << NumberConvertor(readInput("Enter the number: ")) << '\n';
    }
}

Short review: The purpose of code is to tell other programmers what you're telling the computer to do. Your code here reads like you're trying show off all these cool things you can do - which makes for very difficult to read, unimplementable code. Don't perform multiple operations and checks on one line. Don't use member variables to do temporary state. Structure your code to make your intent as readily apparent to the reader as possible.

readInput

Seriously, just no:

while((std::cout << prompt) && (!(std::cin >> answer) || answer < 0 || answer > 99999999))

There is never any reason to write code like that. Why even pass a prompt to the function if you're going to write a different string anyway? I don't see a reason for the prompt.

main

You should write a function that looks like:

std::string convertNumber(int );

Having a streamable class is gratuitous.

NumberConverter

The flow of this class doesn't make much logical sense. You have a member variable mNumber, that you're modifying as you go. It's really more like an implicit argument to convert(). Same with all the member variables actually. Makes it very difficult to reason about the correctness of convert().

words[] doesn't make sense as an array. The three elements in the array have nothing to do with each other. You have the numbers under 20, you have the tens numbers (why the hyphen?) and the block identifiers. Declaring it this way means you have a bunch of cryptic words[2][0] or words[1][index] expressions. Prefer to make three different vectors and name them appropriately.

So convert() takes both a units and its string representation? That's not good design since you're basically passing the same/similar information twice. Prefer to restructure this call such that you're converting one "block" at a time (e.g. the millions block, then the thousands block, then the unit block). Redundant information is bad.

This fragment doesn't make sense either:

std::string word = words[1][index];

if (word[word.size() - 1] == '-')

Why are you using the specific word format as a signal for something? Also, all the words in words[1] end in a hyphen so you always go into this block.


Mobile user agent check I've been using the following user agent Regular Expression to detect mobile devices, but I recently came across a few resources that listed a whole host of mobile user agents that I had not heard of before.

Whilst my agent check is only the first step in the detection process - I also have some JavaScript as a fallback - I would like for it to contain the majority of currently undisputed mobile "keywords" so as to optimise the user experience for those handsets. I would also like to keep things as future-proof as possible, but this can only ever be argued with good reasoning and not set-in-stone.

The core of the regexp follows - please note the user agent string has had it's white-space removed. The check is also specifically case dependent.

Please ignore the newlines; they are for formatting only.

/i[Pp]hone|[Mm]obile[Ss]afari|[Ww]indowsCE|[Ww]indows[Pp]hone|
 IE[Mm]obile|[Oo]pera[Mm]ini|[Oo]pera[Mm]obi|[Bb]lack[Bb]erry|
 [^A-Z]RIM[^A-Z]|[Ss]ony[Ee]ricsson|[Nn]okia|[^A-Z]MIB[^A-Z]/

These are the extra edge-case agents that I've found out about:

/[Ss]kyfire|[Tt]ea[Ss]hark|[Nn]et[Ff]ront|[Mm]inimo|
 [Ii]ris|[Ff]ennec|[Dd]oris|[Ss]eries60|BOLT|[Bb]lazer/

Does anyone have any further suggestions, or possible keywords to remove, that could cause false positives?

For example, I do not include Samsung or Android for the precise reasons that they are not unique to mobile agents. I'd rather let agents through to the JavaScript that I'm unsure about.

As a second note, many user-agents report as Like Something - i.e. Like Mobile Safari - should I filter these out of the equation, or if something is reporting this; is it also likely to be a mobile device.

First of all I guess your first step of the detection process is processed server side because you said there is a fallback which is processed by JavaScript.
Therefore I guess you only perform a HTTP User-Agent check so far.
Suggestion BlackBerry
I would recommend to add also a check of the Accept request header to your first step of detection.
In one of my recent projects I ran into trouble cause the HTTP User-Agent detection of BlackBerry failed in some cases. For these casees I had to add an Accept request header check which looks for /vnd.rim/.
I didn't find out much about the background why it happens. I only know some BlackBerry browsers try to emulate IE or Firefox.
Example
Taken from the BlackBerry 8120 device specification document: application/vnd.rim.html
Suggestion J2ME Devices
Clients using this technology might be quite rare but to be sure you could include another RegEx for midp to cover all kind of MIDP devices.
Question
I know they're dead but I think there are still some around. What about Palm and webOS devices?


Language-detection PHP script I'm working on a webpage with language detection and I have the following script so far (it's simple).  I still haven't done the user detection, so it's not available to find the user language (yet), but this will be easily implemented. Though I'm not asking for that, I'm asking for other ways to improve this code. So far as I've tested it, it's bug-free, but I want it to be bulletproof. How can I improve or expand it?

//LANGUAGES
//Language detection
if ( !empty($_POST['lang']) )
    {
    $Lang = $_POST['lang'];
    $_SESSION['lang']= $_POST['lang'];
    }
else
    {
    if ( !empty ($_SESSION['lang']))
        $Lang = $_SESSION['lang'];
    else
        $Lang = substr ($_SERVER['HTTP_ACCEPT_LANGUAGE'], 0, 2);
    }

//After this, $Lang will have the webpage preference

On principle the code is sound except for one issue: you do not check if the language is "valid". This raises some questions:

How will your localization code behave if I added a lang parameter with the value "foobar"?
How will it behave if I try to do an SQL injection using the parameter value?

In both cases, the sane outcome would be to default to your "main" language, which would probably be hardcoded in your application.

Another issue you may want to consider is that nobody knows that there is an Accept-Language header and how to use it, so using that as a source of the user's preferred locale is not recommended:

  It is not a good idea to use the HTTP Accept-Language header alone to
  determine the locale of the user. If you use Accept-Language
  exclusively, you may handcuff the user into a set of choices not to
  his liking.

In your shoes I would totally scrap using $_SERVER['HTTP_ACCEPT_LANGUAGE'] (IMHO it's useless in practice). If you want to go the extra mile and auto-detect the user's locale, use an IP database (I have used MaxMind GeoIP in the past myself) or the upcoming W3C geolocation spec.


OOP Decision Maker Class - ServerArbiter I am writing a ServerArbiter, the job of which is to make a decision about which server to "choose" given a set of rules. The aim is to be able to add / remove rules at will - for ease of future maintenance. My language of choice is php, so please bear this in mind when answering.

Basically, a server will be chosen depending on various factors. One may be 'server load'. Imagine this arbiter is used to decide which server to add a torrent to - it will choose the server with the least number of torrents in the download queue.

Architecture

My current architecture for this solution consists of a ServerArbiter object:

public function __construct(RuleSet $ruleSet, array $servers);

The RuleSet is effectively just a list of Rule objects that implements \Iterator so it can be foreach()d around.

public function addRule(Rule $rule);

And the actual Rule objects implement the Rule interface:

public function resolve(Server $server);

A Server is just a (doctrine) Entity - that is, it contains class members, setters and getters, and some validation. These properties are populated with data from the servers database table. One of the properties is ipAddress.

Rules

I currently have rules such as the ServerActiveRule, which is just a property that is returned.

class ServerActiveRule implements Rule
{
    /**
     * {@inheritdoc}
     */
    public function resolve(Server $server)
    {
        return $server->isActive();
    }
}

For torrents example described above, I was going to have a ServerDownloadsComparisonRule:

class ServerDownloadsComparisonRule
{
    /**
     * @var Server[]
     */
    private $servers;

    public function __construct(array $servers)
    {
        // -- SNIP --
        // Make sure each $server is instance of Server entity
        // -- SNIP --
    }

    public function resolve(Server $server)
    {
        // I'm guessing this is where I will compare the downloads to get the one with the least number of downloads in
        // I haven't implemented this yet
    }
}

ServerArbiter::decide()

Finally, this is the current method I have of deciding which Server entity to return.

/**
 * Run each Server against each Rule in the RuleSet and return the Server that matched the most rules
 *
 * @return Server The one Server to rule them all
 *
 * @note If two servers match the same number of rules successfully, the first one will be returned
 */
public function decide()
{
    $decisions = [];

    /** @var Rule $rule */
    foreach ($this->ruleSet as $rule)
    {
        foreach ($this->servers as $server)
        {
            if (!array_key_exists($server->getId(), $decisions))
            {
                $decisions[$server->getId()]['successes'] = 0;
            }

            if ($rule->resolve($server))
            {
                $decisions[$server->getId()]['successes']++;
            }
        }
    }

    $decision = array_search(max($decisions), $decisions);

    return $decision;
}

Questions

Simple rules that return true / false are fine. But as soon as ServerDownloadsComparisonRule gets thrown into the mix, it has a different constructor requiring an array of servers. This spells mayhem for the ServerArbiter::decide() method as it's now doing a lot more work. How can I refactor all this to make it much simpler to let each rule individually do what it needs to do?
Obviously my rule for checking downloads will have to connect to the server via it's IP address. Is it okay to Inject a dependency to perform this data retrieval just for the rule?
How is the architecture, what can I do differently? What is stupid about it?
Is there anything else that I can do better?

Extra Info

I'm using a recursively instantiating dependency injection container. This means I can typehint for any object anywhere in my application, including in this service, and the object will automatically be passed in for me. Therefore, I don't have to worry about wiring - it needs to be more SOLID and simple to modify in the future.

Why does the decide() method do more work? It's the problem of the object constructing your Rules, which is not necessarily the ServerArbiter.
Isn't that already a part of the Server entity? Why do you need to inject yet another dependency. If the server's address isn't in the Server entity, why the hell the server's address isn't in the Server entity?!
Your architecture is fine. Easily separated, Servers don't know about the Rules.
Yes. Since querying all the servers to get their downloads count, it makes sense to remember the best server in the ServerDownloadsComparisonRule the first time resolve is called, and then just reference it from the cache.

if ($bestServer == null) {
    $bestServer = $this->findServerWithLeastDownloads();
}
return $server === $bestServer;


Optimize printing massive strings to System.out with respect to time I need to cut the time it takes to print a large string by ~50%.

I went from using s1 + s2 to using StringBuilder, and instead of making several calls to System.out.print for each line, I create one large string by appending newlines to the StringBuilder, and then print that large string just once with System.out.

But my code is still way too slow (this is for a homework assignment and the code is not accepted by the automated hand-in system because it takes too long).

When the StringBuilder is printed, the string has ~100k lines

The relevant part of my code looks like this:

public class Main {
    public static void main (String[] args){
        MyClass builder = new MyClass();
        new ClassThatBuildsOnBuilder(builder);
        System.out.println(builder.str.toString());
    }
}

public class MyClass {
    private double[] coordinates = new double[]{0, 0, 0, 0};

    public StringBuilder str = new StringBuilder();
    private DecimalFormat formatter = new DecimalFormat("#0.0000");

    public MyClass(){

    }

    public void changeValues(double[] coordinates){
        this.coordinates = coordinates;
        updateRow();
    }

    private void updateRow(){
        for (double d : coordinates){
            str.append(formatter.format(d));
            str.append(" ");
        }
        str.append("\n");
    }

Can this even be optimized further? I feel like this should be very close to a lower bound.

Are you sure the bottleneck is the printing, as opposed to whatever new ClassThatBuildsOnBuilder(builder); is doing?
Is coordinates always going to have a width of four?
It appears you're building the whole string in memory, and then printing all your output at once at the end. While printing each line to standard-out individually may be slower, it also may take better advantages of the built-in output buffering. (Printing each word individually as you describe having originally done probably isn't a great idea.

Would something like this work?

  private void printRow(){
    System.out.format("#%.3f #%.3f #%.3f #%.3f%n",
      coordinates[0],
      coordinates[1],
      coordinates[2],
      coordinates[3]);

You could also try a Formatter, which would let you use a StringBuilder as you already are, but also use printf syntax.

Finally, I don't know how to take manual control of System.out's buffering behavior, but it's probably possible and may help.


Cleaner way of returning true/false with error message What's a concise way of returning true or false along with an error message?

Here's a concrete example - I'm building a parser that takes in a file. ParseReplayData does some validation before proceeding with the actual parsing:

public class Parser
{
    private const string SUPPORTED_FILE_EXTENSION = "w3g";
    private const string ERROR_FILE_DOES_NOT_EXIST = "Replay file does not exist: {0}";
    private const string ERROR_FILE_EXTENSION_NOT_SUPPORTED = "File extension is not a w3g replay file: {0}";
    private const string ERROR_FILE_BYTE_LENGTH_TOO_SMALL = "Replay file's byte length is too small: {0}";
    private const int REPLAY_MINIMUM_BYTE_LENGTH = 288;
    private ReplayData _replayData;
    private readonly string _replayFilePath;

    public ReplayData ReplayData
    {
        get { return _replayData; }
    }

    public Parser(string replayFilePath)
    {
        _replayFilePath = replayFilePath;
    }

    public bool ParseReplayData(out string errorReason)
    {
        errorReason = String.Empty;
        if (!File.Exists(_replayFilePath))
        {
            errorReason = String.Format(ERROR_FILE_DOES_NOT_EXIST, _replayFilePath);
            return false;
        }
        if (Path.GetExtension(_replayFilePath) != SUPPORTED_FILE_EXTENSION)
        {
            errorReason = String.Format(ERROR_FILE_EXTENSION_NOT_SUPPORTED, _replayFilePath);
            return false;
        }

        byte[] replayFileBytes = File.ReadAllBytes(_replayFilePath);
        if (replayFileBytes.Length < REPLAY_MINIMUM_BYTE_LENGTH)
        {
            errorReason = String.Format(ERROR_FILE_BYTE_LENGTH_TOO_SMALL, _replayFilePath);
            return false;
        }
        _replayData = new ReplayData();
        ..... //Parse Replay
        return true;
    }
}

I can't shake the feeling that there's a lot of code smell in this class, and I'd appreciate any other suggestions you could give me.

An alternative as MrSmith42 suggested could be to use exceptions.

public class ReplayDataParser
{
    private const string SupportedFileExtension = "w3g";
    private const string ErrorFileDoesNotExist = "Replay file does not exist: {0}";
    private const string ErrorFileExtensionNotSupported = "File extension is not a w3g replay file: {0}";
    private const string ErrorFileByteLengthTooSmall = "Replay file's byte length is too small: {0}";
    private const int ReplayMinimumByteLength = 288;

    private readonly string _replayFilePath;

    public ReplayDataParser(string replayFilePath)
    {
        _replayFilePath = replayFilePath;
    }

    public ReplayData Parse()
    {
        if (FileNotFound())
        {
            throw new FileNotFoundException(string.Format(ErrorFileDoesNotExist, _replayFilePath));
        }

        if (FileExtensionNotSupported())
        {
            throw new ArgumentException(string.Format(ErrorFileExtensionNotSupported, _replayFilePath));
        }

        if (FileNotTooSmall())
        {
            throw new FileTooSmallException(string.Format(ErrorFileByteLengthTooSmall, _replayFilePath));
        }

        return ParseFile();
    }

    private ReplayData ParseFile()
    {
        return new ReplayData();
    }

    private bool FileExtensionNotSupported()
    {
        return Path.GetExtension(_replayFilePath) != SupportedFileExtension;
    }

    private bool FileNotFound()
    {
        return !File.Exists(_replayFilePath);
    }

    private bool FileNotTooSmall()
    {
        byte[] replayFileBytes = File.ReadAllBytes(_replayFilePath);
        return replayFileBytes.Length < ReplayMinimumByteLength;
    }
}

The downside from this from what I can see is if you use this parser in alot of places then you will need to handle the exceptions accordingly.  However the upside is that if you wish to do different things based on the reason why something failed then you can catch individual exceptions as required.

Offered as an alternative for consideration in any event.


Number aware string sorting with comparator I have this class for use in sorting strings such that if strings have a number in the same position it will order the numbers in increasing order.

Alphabetical gives:

file1
file10
file2

What I'm calling "number aware" string sorting should give:

file1
file2
file10

Here is what I have using a regex split from there.

The code seems to work. Any cases where I could run into problem? If not any suggestions on making it simpler or more efficient.

import java.util.Comparator;

public class NumberAwareStringComparator implements Comparator<String>{
     public int compare(String s1, String s2) {

            String[] s1Parts = s1.split("(?<=\\D)(?=\\d)|(?<=\\d)(?=\\D)");
            String[] s2Parts = s2.split("(?<=\\D)(?=\\d)|(?<=\\d)(?=\\D)");

            int i = 0;
            while(i < s1Parts.length && i < s2Parts.length){

                //if parts are the same
                if(s1Parts[i].compareTo(s2Parts[i]) == 0){
                    ++i;
                }else{
                    try{

                        int intS1 = Integer.parseInt(s1Parts[i]);
                        int intS2 = Integer.parseInt(s2Parts[i]);

                        //if the parse works

                        int diff = intS1 - intS2;
                        if(diff == 0){
                            ++i;
                        }else{
                            return diff;
                        }
                    }catch(Exception ex){
                        return s1.compareTo(s2);
                    }
                }//end else
            }//end while

            //Handle if one string is a prefix of the other.
            // nothing comes before something.
            if(s1.length() < s2.length()){
                return -1;
            }else if(s1.length() > s2.length()){
                return 1;
            }else{
                return 0;
            }
        }
}

Exceptions should be reserved for exceptional situations, and should be avoided if possible.  The fundamental reason you have to deal with NumberFormatException is that after splitting, you don't know whether each part contains a number or a non-number.

Here's a strategy that always compares non-digits to non-digits, and numbers to numbers.

import java.math.BigInteger;
import java.util.Comparator;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

public class NumberAwareStringComparator implements Comparator<CharSequence> {
    public static final NumberAwareStringComparator INSTANCE =
        new NumberAwareStringComparator();

    private static final Pattern PATTERN = Pattern.compile("(\\D*)(\\d*)");

    private NumberAwareStringComparator() {
    }

    public int compare(CharSequence s1, CharSequence s2) {
        Matcher m1 = PATTERN.matcher(s1);
        Matcher m2 = PATTERN.matcher(s2);

        // The only way find() could fail is at the end of a string
        while (m1.find() && m2.find()) {
            // matcher.group(1) fetches any non-digits captured by the
            // first parentheses in PATTERN.
            int nonDigitCompare = m1.group(1).compareTo(m2.group(1));
            if (0 != nonDigitCompare) {
                return nonDigitCompare;
            }

            // matcher.group(2) fetches any digits captured by the
            // second parentheses in PATTERN.
            if (m1.group(2).isEmpty()) {
                return m2.group(2).isEmpty() ? 0 : -1;
            } else if (m2.group(2).isEmpty()) {
                return +1;
            }

            BigInteger n1 = new BigInteger(m1.group(2));
            BigInteger n2 = new BigInteger(m2.group(2));
            int numberCompare = n1.compareTo(n2);
            if (0 != numberCompare) {
                return numberCompare;
            }
        }

        // Handle if one string is a prefix of the other.
        // Nothing comes before something.
        return m1.hitEnd() && m2.hitEnd() ? 0 :
               m1.hitEnd()                ? -1 : +1;
    }
}

Since strings of digits (such as those representing dates, like 20131212123456.log) can overflow an int, I've used java.math.BigInteger.

Also, since the code works just as well with CharSequence as with String, I've generalized the type to Comparator<CharSequence>.


JavaScript portion of regexp tester I'm quite new to JavaScript, and I'd like a review of the code structure and syntax. It serves this little online regexp test (still a work in progress).

The whole code (JavaScript, CSS & HTML) is on GitHub.

application.js:

function update_result_for(input, regexp_value) {
  var input_value   = input.val();
  var result_spans  = input.parent().children('span');
  if(!input_value || !$('#regexp').val()) {
    result_spans.hide();
  } else {
    var regexp = new RegExp(regexp_value);
    var result = regexp.exec(input_value);
    if(result) {
      var matched_string = result.shift();

      var submatches_list_string = jQuery.map(result, function(submatch, index) {
        return '$' + (index + 1) + ' = ' + submatch;
      }).join('; ');

      var regexp_to_highlight_matched_string          = new RegExp('(.*)' + matched_string + '(.*)');
      var regexp_to_highlight_matched_string_result   = regexp_to_highlight_matched_string.exec(input_value);
      var before_matched_string                       = regexp_to_highlight_matched_string_result[1];
      var after_matched_string                        = regexp_to_highlight_matched_string_result[2];
      var input_value_with_matched_string_highlighted   = 'matched: ' + before_matched_string + '' + matched_string + '' + after_matched_string;

      result_spans.filter(".submatches").text(submatches_list_string);
      result_spans.filter(".match").html(input_value_with_matched_string_highlighted);
      result_spans.filter(".ok").show('fast');
      result_spans.filter(".not_ok").hide();
    } else {
      result_spans.filter(".not_ok").show('fast');
      result_spans.filter(".ok").hide();
    }
  }
}

// from http://www.scottklarr.com/topic/126/how-to-create-ctrl-key-shortcuts-in-javascript/
var isCtrl = false;
$(document).keyup(function (e) {
    if(e.which === 17) isCtrl=false;
}).keydown(function (e) {
    if(e.which === 17) isCtrl=true;
    if(e.which === 69 && isCtrl) {
    $('#regexp').focus();
        return false;
    }
});

$(document).ready(function() {
  $('#regexp').focus();
  $('span.result').hide();
  $('input:not(#regexp)').live("keyup", function() {
    update_result_for($(this), $('#regexp').val());
  });
  $('input#regexp').keyup(function() {
    $('input:not(#regexp)').each(function(i) {
      update_result_for($(this), $('#regexp').val());
    });
  });
  $('a.add_example').click(function() {
    new_example = $('div#examples p:last').clone();
    new_example.children('input').attr('value', '');
    new_example.children('span').hide();
    new_example.insertBefore($(this));
    new_example.children("input").focus();
  });
});

This code looks fine to me for a toy project. I especially appreciate the long names for identifiers.

Now, it could look better:

with more comments. You should describe the intent (the why behind the how) and the expected use case for the function: type and range of values in parameters, one line to describe what it does, and one more paragraph for details if needed.
my personal preference would be to use camelCase instead of underscore_between_words for long names. Although this is open to discussion.
a little trick: end the if branch with return; to avoid nesting all remaining code in the else branch:

function update_result_for(input, regexp_value) {
  var input_value   = input.val();
  var result_spans  = input.parent().children('span');
  if(!input_value || !$('#regexp').val()) {
    result_spans.hide();
    return; // return as soon as possible to avoid deep nesting
  }
  // no need for else
  var regexp = new RegExp(regexp_value);
  var result = regexp.exec(input_value);
  // ...
}

in the same vein, treat exceptional cases first and normal cases after. This is a useful convention which helps the reader, and the exception handling is usually shorter (or should be extracted to a separate function if longer) which avoids long runs of nested code:

if (!result) {
  result_spans.filter(".not_ok").show('fast');
  result_spans.filter(".ok").hide();
  return;
}
// reduced nesting
var matched_string = result.shift();
// ...

only use anonymous functions when you actually need a closure with access to the context: the intent of the function will be clearer with a name, you will avoid nesting, and may reuse the function more easily including for unit testing. There are many anonymous functions in your code, which makes it harder to understand:

function(submatch, index) {
  return '$' + (index + 1) + ' = ' + submatch;
})

function(e) {
  if(e.which === 17) isCtrl=false;
}

function(e) {
  if(e.which === 17) isCtrl=true;
    if(e.which === 69 && isCtrl) {
    $('#regexp').focus();
        return false;
    }
})

function() {
  $('#regexp').focus();
  $('span.result').hide();
  // ...
}

function() {
  update_result_for($(this), $('#regexp').val());
}

function() {
  $('input:not(#regexp)').each(function(i) {
    update_result_for($(this), $('#regexp').val());
  });
}

function() {
  new_example = $('div#examples p:last').clone();
  new_example.children('input').attr('value', '');
  new_example.children('span').hide();
  new_example.insertBefore($(this));
  new_example.children("input").focus();
}

once you define more than one function, you should wrap your code in a closure to avoid cluttering the global namespace, following the Module Pattern:

(function(){
  // private scope for your code
}());

break long lines to fit in about 80 characters to avoid the need for scrolling horizontally in typical console windows and in code areas on this site:

var regexp_to_highlight_matched_string =
  new RegExp('(.*)' + matched_string + '(.*)');
var regexp_to_highlight_matched_string_result =
  regexp_to_highlight_matched_string.exec(input_value);
var before_matched_string =
  regexp_to_highlight_matched_string_result[1];
var after_matched_string =
  regexp_to_highlight_matched_string_result[2];
var input_value_with_matched_string_highlighted =
  'matched: ' +
  before_matched_string +
  '' + matched_string + '' +
  after_matched_string;

To go further, my advice would be to read "JavaScript: The Good Parts" and start using JSLint, in this order: this is an enlightening experience on your way to mastering JavaScript. The other way round, using the tool without understanding the mindset of its author, is very frustrating.

I ran JSLint on your code. It has one critical complaint hidden among hair splittings: the declaration of new_example is missing, it is therefore a global variable which is susceptible to result in unexpected bugs.

   // var keyword added:
   var new_example = $('div#examples p:last').clone();


Proper capitalization for book titles I'm working through some exercises to sharpen my Ruby skills. The following code is how I solved this particular question. I'd like to know what other developers think of this solution. I'm self taught, which means: I'm not getting any feedback as I learn, so I just want to make sure that I'm learning the right way to do things. And I'd like to get a gauge of what professional developers think of my code.

  Ranking System
  [1-terrible] [2-not good] [3-ok] [4-good] [5-very good]

Is the answer code elegant? (score and feedback)
What tips can you suggest to improve the answer?
How's the readability of the code? (score and feedback)
Can you suggest a simpler and alternative answer?

Ruby

class Book
  def title
    @title
  end

  def title=(title)
  special_words = %w(and in the of a an)
  formatted_title = []

    @title = title.split.each_with_index do |w,i|
      case
      when i == 0
        formatted_title << w.capitalize

      when i > 0 && !special_words.include?(w)
        formatted_title << w.capitalize

      when special_words.include?(w)
        formatted_title << w
      end
    end

  @title = formatted_title.join(" ")
  end
end

Rspec

describe Book do

  before do
    @book = Book.new
  end

  describe 'title' do
    it 'should capitalize the first letter' do
      @book.title = "inferno"
      @book.title.should == "Inferno"
    end

    it 'should capitalize every word' do
      @book.title = "stuart little"
      @book.title.should == "Stuart Little"
    end

    describe 'should capitalize every word except...' do
      describe 'articles' do
        specify 'the' do
          @book.title = "alexander the great"
          @book.title.should == "Alexander the Great"
        end

        specify 'a' do
          @book.title = "to kill a mockingbird"
          @book.title.should == "To Kill a Mockingbird"
        end

        specify 'an' do
          @book.title = "to eat an apple a day"
          @book.title.should == "To Eat an Apple a Day"
        end
      end

      specify 'conjunctions' do
        @book.title = "war and peace"
        @book.title.should == "War and Peace"
      end

      specify 'prepositions' do
        @book.title = "love in the time of cholera"
        @book.title.should == "Love in the Time of Cholera"
      end
    end

    describe 'should always capitalize...' do
      specify 'I' do
        @book.title = "what i wish i knew when i was 20"
        @book.title.should == "What I Wish I Knew When I Was 20"
      end

      specify 'the first word' do
        @book.title = "the man in the iron mask"
        @book.title.should == "The Man in the Iron Mask"
      end
    end
  end
end

Your code is readable, but it doesn't feel very "rubyish" to me.   I think that's due to needing to setup and track some extra variables and track the index of the word in question -- having to case it depending on the index, etc.

It also bugs me a little bit that title= is doing so much.  This is how I would have written it:

class Book
  def title
    @title
  end

  def title=(title)
    @title = titlieze(title)
  end

  private
  def titlieze(title)
    stop_words = %w(and in the of a an)
    title.capitalize.split.map{|w| stop_words.include?(w) ? w : w.capitalize}.join(' ')
  end

end

I used stop_words instead of special_words since that's a common naming scheme for search applications (ie. words you ignore when searching).  I'd be tempted to move that into a constant or some other configuration, but it works fine for this.

Rspec output:

$ rspec -f d foo_spec.rb

Book
  title
    should capitalize the first letter
    should capitalize every word
    should capitalize every word except...
      conjunctions
      prepositions
      articles
        the
        a
        an
    should always capitalize...
      I
      the first word

Finished in 0.00261 seconds
9 examples, 0 failures


Adding, deleting elements as per autocomplete results - updated I am working on a Python/Django project which has turned into a JavaScript/jQuery project due to the amount of asynchronous functionality that I thought to add.
There is a JavaScript file which is doing everything for the JS side. I am a newbie in JavaScript/jQuery and am starting to think that this is going to become a big problem soon.
I asked for a previous review but as I never got one there and the file has changed a lot since then I am placing part of the file where I think something can be be improved.

Some functions which are being used by the code:

function bind_events(cur_obj, e) {
    var trigger_name = "";
    if (e.keyCode == 13) {
        trigger_name = EV_ENTER_KEY;
    } else if (e.altKey && e.keyCode == "N".charCodeAt(0)) {
        trigger_name = EV_ALT_N;
    }
    $(cur_obj).trigger(trigger_name);
}

//Auxiliary functions for categories
function get_all_categories() {
    var class_ids = [];
    $(CLASS_DEL_CATEGORY).each(function () {
        class_ids.push(get_class_string(this, 11));
    });
    return class_ids.join(",");
}

function enter_category(category_box, trigger_event, trigger_data) {
    if (value_in_selector(category_box, CLASS_UI_MENU_ITEM) == false) {
        alert(MSG_NOT_CATEGORY);
    } else if (value_in_selector(category_box, CLASS_CATEGORY) == true) {
        alert(MSG_CATEGORY_ADDED);
        category_box.val("");
    } else {
        category_box.trigger(trigger_event, trigger_data);
    }
}

function add_category(cur_obj, e, data) {
    $.ajax({
        url: URL_CATEGORY,
        data: {'value': $(cur_obj).val()},
        success: function (output) {
            data.cat_list.append(output);
            $(cur_obj).val("");
            if (data.trigger_obj !== undefined) {
                data.trigger_obj.trigger(data.next_trig);
            }
        }
    });
}

The code that I think needs to be improved is below.

//Actions for Search's category box
CATEGORY_INPUT.on({
    keyup: function (e) {
        bind_events(this, e);
    },

    "enterKey": function () {
        enter_category($(this), EV_ADD_CATEGORY, [
            {
                cat_list: CATEGORY_LIST_SEARCH,
                trigger_obj: BOOKMARK_LIST,
                next_trig: EV_UPDATE_BOOKMARKS
            }
        ])
    },

    "addCategory": function (e, data) {
        add_category(this, e, data);
    },

    "clearAll": function () {
        $(CLASS_CATEGORY).remove();
        $(this).val("");
        BOOKMARK_LIST.trigger(EV_CLEAR_ALL);
    }
});

//Actions for Add Bookmark's category box
CATEGORY_BOX.on({
    keyup: function (e) {
        bind_events(this, e);
    },

    "enterKey": function () {
        enter_category($(this), EV_ADD_CATEGORY, [
            {
                cat_list: CATEGORY_LIST_ADD,
                trigger_obj: undefined,
                next_trig: undefined
            }
        ])
    },

    "addCategory": function (e, data) {
        add_category(this, e, data);
    }
});

As you can see nearly same events are bound to both with minor changes. I thought to bind events to multiple selectors but as not all are same events readability will suffer.

Any thoughts for improvements?

Structurally, it doesn't look too bad, although it seems inconsistent.

By convention, JavaScript uses camelCase naming - not snake_case. All your code is snake_case, but then you have things like keyCode and charCodeAt because that's what JS's own APIs use. So I'd encourage you to switch you code to camelCase in keeping with JS conventions.

You've got a number of "constants" (in quotes, since JS doesn't actually have constants), like EV_ENTER_KEY, which, judging from your comment, is a string (enterKey in this case). Sometimes you use the constants by name, other times you use them by value.

While it's true you can't do this:

CONST = "foo";
obj = {
  CONST: "bar"
};

and get an object like {foo: "bar"} (because JS interprets the CONST: literally), you can do this:

obj = {}
obj[CONST] = "bar";

And your object will be {foo: "bar"}.

So that would clean things up, and allow you to actually use constants. E.g.

categoryBoxEvents = {}
categoryBoxEvents.keyup = function (e) {
  bind_events(this, e);
};

categoryBoxEvents[EV_ENTER_KEY] = function () {
  enter_category($(this), EV_ADD_CATEGORY, [{
    cat_list: CATEGORY_LIST_ADD,
    trigger_obj: undefined,
    next_trig: undefined
  }]);
};

categoryBoxEvents[EV_ADD_CATEGORY] = function (e, data) {
  add_category(this, e, data);
};

CATEGORY_BOX.on(categoryBoxEvents);

However, don't rely on undefined being undefined. For mysterious reasons, undefined isn't a reserved word in JS, meaning that some (older) runtimes allow it be defined. Instead, use null or just don't set a value at all.

For instance, the event-key event handler above could simply be:

enter_category($(this), EV_ADD_CATEGORY, [{
  cat_list: CATEGORY_LIST_ADD
}]);

If you don't define trigger_obj and next_trig, they're - by definition - undefined.

Where you need to to check for these values, you can simply do

if (!data.trigger_obj) { ... }

because undefined is false'y (and so is false, null, 0, and "").

Or, if you feel you should be (needlessly) exact:

if (typeof data.trigger_obj === 'undefined') { ... }

I'd also skip a lot of the proxy-functions you've got going on. For instance, add_category is called from 2 identical wrapper functions, only so its 1st argument can be set. Don't bother. Try this instead:

function add_category(e, data) {
  var $this = $(this); // just use `this`
  $.ajax({
    url: URL_CATEGORY,
    data: { value: $this.val() },
    success: function (output) {
      data.cat_list.append(output);
      $this.val("");
      if(data.trigger_obj && data.next_trig) {
        data.trigger_obj.trigger(data.next_trig);
      }
    }
  });
}

// ... snip ...

categoryBoxEvents[EV_ADD_CATEGORY] = add_category;

Now, when add_category is called, its this will automatically be the right object. No need for you to specify it. And if you ever need to call add_category in a different context, you can use call() or apply(), or bind it to an object with bind() (not supported on all browsers) or jQuery.proxy().

Lastly, I think you may be a bit trigger-happy (pardon the pun). You're handling events by triggering a lot of other events, which in turn seem to trigger events and so on. While that does decouple your code, it also introduces some overhead. It might make the code simpler to skip some of the trigger-cascades in favor of just calling functions directly. Same goes for the constants, which, while useful in many cases, seem a little excessive. Functions are first-class objects, so you can simply pass them around instead of referring to them.


Reverse a word using string manipulation #include<stdafx.h>
#include<stdio.h>
#include<stdlib.h>
#include<string>
#include<iostream>
using namespace std;
string temp[10];
void extract(char * c)
{
    int ind = 0;

    //char *temp = (char *)malloc(sizeof(char));
    while(*c!= NULL)
    {
        if(*c!= ' ') // read all chars in a word and store in temp.
        {
        temp[ind]= temp[ind] + *c;
        c++;

        }
        else if(*c== ' ') // reached end of word
        {
        c++;
        ind++;
        }
    }
    int length = sizeof(temp)/sizeof(temp[0]);
    printf("The reversed string is");
    for(int i=length;i>=0;i--)
    {
    cout<<temp[i];// not able to print it with printf statement
    }

}

void main()
{
char *c = "seattle is good";
extract(c);

}

This should print the sentence in reverse order . Input is "seattle is good"
Output for the above will be "good is seattle"
I have 2 questions ,
1. I am using string array here with size of 10. How can I make it dynamic?
2. For some reason the printf("%s",temp[i]) was not giving me correct results(At the very end of extract method). I then used cout and it was fine. Any ideas why? Am I doing something silly?

Algorithm Description:

Take the input string as char array.
Extract words from the string by looking for empty space.
Store each word in a temp string array.
temp[0] = seattle
temp[1] = is
temp[2] = good
Then loop backwards through this temp array and print contents.

In response to your specific questions:

Use a vector. Using a vector you can just append to the end and it will resize itself as necessary.
You can not use printf with strings as printf is a C function and does not know C++'s strings. Either way you should generally avoid printf in C++ code.

General notes on your code:

#include<stdafx.h>
#include<stdio.h>
#include<stdlib.h>

The correct way to include C header files in C++ is to use #include <cheadername>, not #include <headername.h>. Further you should avoid C-functions when there are good C++ alternatives and you should generally not mix C's stdio with C++'s iostream. In this particular program I do not believe you need any of the C headers you include.

string temp[10];

I don't see any reason to have your global storage as a global variable. You should move it as a local variable inside the extract method. And as I already said, you should use a vector instead of a C array.

Also temp is a bad variable name.

void extract(char * c)

Don't mix char*s and strings unless necessary. In this case you can use strings all the way, so you should define extract with const string& sentence as an argument (note that sentence is also a more descriptive name than c).

Further it seems that your extract function is doing too much work: it's extracting words from the sentence into an array, and then printing it in reverse. These two separate steps should be done by two functions: extract (or maybe sentence_to_words which is more descriptive) and print_in_reverse.

You might even want to use 3 steps instead: extract the words, reverse them, then print them. This way your IO code is completely separate from any logic, which is always good. As a bonus point C++ already has a built-in function to reverse an array or vector, so no additional work for you.

Either way you'd need to change your extract function to either return the extracted words rather than void or take a vector into which to store the extracted words as an argument (by reference).

//char *temp = (char *)malloc(sizeof(char));

This code is commented out anyway (and doesn't do anything sensible from the looks of it), but as a general principle I want to point out that there's rarely a reason to use malloc over new in C++.

int ind = 0;
while(*c!= NULL)
{
    if(*c!= ' ') // read all chars in a word and store in temp.
    {
    temp[ind]= temp[ind] + *c;
    c++;

    }
    else if(*c== ' ') // reached end of word
    {
    c++;
    ind++;
    }
}

Using a vector you can get rid of ind as you can just push_back into the vector.

Further if you use strings, you can simplify this part by using find to find the next space and substr take the substring up to that space.

int length = sizeof(temp)/sizeof(temp[0]);

Note that this will only ever tell you the size of an array if the array's size is known at compile time (which is more or less equivalent to: "spelled out in the code"). So if you want the size of the array to be dynamic, you can't use this. If you use a vector, you can just use temp.size().

printf("The reversed string is");

As I already said, you shouldn't mix stdio and iostream. There is no reason to use printf here.

void main()

The correct return value for main is int, not void.

char *c = "seattle is good";

Again this should just be string sentence("seattle is good");. No reason to use char*.

If I were to write this program, I might do it like this:

#include <string>
#include <iostream>
#include <string>
#include <iostream>
#include <vector>
#include <algorithm>

using std::string;
using std::vector;
using std::cout;
using std::endl;
using std::reverse;

// Splits a sentence by spaces into a vector of words
void extract(const string& sentence, vector<string>& words)
{
    size_t pos = 0;
    while(pos != string::npos)
    {
        // Find position of next space
        int next_space = sentence.find(" ", pos);
        // Store everything up to next space in the words vector
        words.push_back( sentence.substr(pos, next_space - pos));

        // Continue at the next character which is not a space.
        pos = sentence.find_first_not_of(" ", next_space);
    }
}

// Prints the strings in the vector separated by spaces
void print_strings(const vector<string>& strings)
{
    for(size_t i = 0; i < strings.size(); i++) {
        cout << strings[i] << " ";
    }
    cout << endl;
}

int main()
{
    string sentence("seattle is good");
    vector<string> words;
    extract(sentence, words);
    reverse(words.begin(), words.end());
    print_strings(words);
}


PHP subset of 2 arrays containing objects - compared only by 1 attribute I'm looking for a fast way to get the subset of 2 arrays containing the same kind of objects using PHP functions.

I'm looking for the red part:

class Order
{
 private $_orderId;
 private $_someOtherAttributes;
}

I'm currently using array_uintersect() to get the intersection and array_udiff() to get rid of the intersection.

$array_1 = array($o1, $o2, $o3 ... $on);
$array_2 = array($o1, $o2, $o3 ... $on);

function order_hash($object)
{
    return sprintf('%d', $object->getOrderId());
}

 function compare_objects($a, $b)
 {
      return strcmp(order_hash($a), order_hash($b));
 }

$intersection = array_uintersect($array_1, $array_2, 'compare_objects');
$subset = array_udiff($array_2, $intersection, 'compare_objects');

Is there a faster / more efficient way? I need to work with a lot of objects > 400 each array and very often as well.

I'm currently using PHP Version 7.1.4.

My changes:

I changed the Order-Class to:

class Order
{
     public $_orderId;
     private $_someOtherAttributes;
}

and compare_objects() to:

function compare_objects($a, $b)
{
       if($a->_orderId> $b->_orderId)
       {
           return 1;
       }
       else if($a->_orderId< $b->_orderId)
       {
            return -1;
       }
       return 0;
}

and using array_udiff($array_1, $array_2, 'compare_objects') instead off array_udiff($array_2, array_udiff($array_1, $array_2, 'compare_objects'), 'compare_objects')

I'm aware that public attributes aren't the best practice, but changing it this way I gained 300% more speed.

As far as these things go, I think your performance is going to be close to ideal.  There is maybe a little room for improvement, namely:

I'm pretty sure all you need is array_udiff.  My suspicion (and some quick tests) suggest that this:

$subset = array_diff( $array_2, array_intersect( $array_1, $array_2 ) );

Is exactly equivalent to this:

$subset = array_diff( $array_2, $array_1 );

(some details excluded for brevity) So save yourself an expensive function call.  My only other suggestion would be a more "global" change that may or may not be helpful.  In your example it looks like you don't care about the keys.  In that case, use them.  When you build your array of objects use order_hash( $obj ) as the key.  They you can do array_diff on the array_keys, without having to worry about compare_objects method, i.e.:

$array1 = [
    order_hash( $o1 ) => $o1,
    order_hash( $o2 ) => $o2,
];

$array2 = [
    ... same thing ...
]

$keys_to_check = array_diff( array_keys( $array2, $array1 ) );
foreach ( $keys_to_check as $key )
    do_something( $array2[$key] );

It is a different way of organizing your data structures, and it takes a little more awareness about how your objects are stored.  However, if these arrays of objects are being passed back and forth frequently, and if you are frequently doing these kinds of comparisons, it can save you a lot of time (I think) because you only have to do your hash calculation once per object, instead of doing it once per object per intersect.

I frequently organize my structures this way (and do lots of diffs on them), and I find it easier to follow.  Basically, the thinking changes from:

Use this function to compare these arrays and see which ones are different

to

Check the keys of this array to see which ones are different

Personally I prefer the latter.


Optimizing a binary search algorithm I have implemented a binary search algorithm in C#. Can anyone give me suggestions on improving its effectiveness?

using System;

namespace bs2
{
    class Program
    {
        static void Main(string[] args)
        {
            int n = int.Parse(Console.ReadLine());
            int[] arr = new int[n];

            for(int i=0;i<n;i++)
            {
                arr[i] = i*14; //14 is a random number, I multiply by a number so that arr[i] would not be equal to i.
            }

            int searchingfor = int.Parse(Console.ReadLine());
            int min = 0;int max = n-1;int mid = (n-1)/2;
            while (true)
            {
                if (arr[mid] == searchingfor)
                {
                    Console.WriteLine("Element is on position {0}", mid);
                    break;
                }else if (arr[mid] < searchingfor)
                {
                    min = mid;
                    mid = (min + max) / 2;
                }else if (arr[mid] > searchingfor)
                {
                    max = mid;
                    mid = (min + max) / 2;
                }
            }

        }
    }
}

The core of your search algorithm is OK except for one thing:

The while(true) loop will run forever, if the searchingFor is not in the array.

In addition you should organize your code properly by separating functionality:  test input/output in one function and the algorithm in another:

void TestBinarySearch()
{
  Console.Write("Enter Array Length: ");
  int length = int.Parse(Console.ReadLine());
  Console.Write("Enter Search Target: ");
  int target = int.Parse(Console.ReadLine());

  Random rand = new Random(5);
  int[] data = Enumerable.Range(0, length).Select(n => rand.Next(length)).OrderBy(d => d).ToArray();
  int foundIndex = BinarySearch(data, target);
  if (foundIndex >= 0)
    Console.WriteLine($"Target found at: {foundIndex}");
  else
    Console.WriteLine("Target not found");
}

int BinarySearch(int[] data, int target)
{
  int min = 0;
  int max = data.Length - 1;
  int mid = (data.Length - 1) / 2;

  while (min != mid && max != mid)
  {
    if (data[mid] == target)
    {
      return mid;
    }

    if (data[mid] < target)
    {
      min = mid;
      mid = (min + max) / 2;
    }
    else if (data[mid] > target)
    {
      max = mid;
      mid = (min + max) / 2;
    }
  }

  return -1;
}


Wrapping text in  tags I needed a script to wrap phrases with  tag, they all are separated by commas, for i.e "Alan, Bob, John".

I came to this result:

        $actors = explode(',', $item->casting);

        $item->casting = array_map(function($a) {
            $a = trim($a);
            return ''.$a.'';
        }, $actors);

        echo implode(' ', $item->casting);

I'd like to know is there a better way achieve this result ?

Your code doesn't handle names with spaces within, nor names with characters that are illegal for URL's. To accomodate these two issues, you could use something like:

$item->casting = 'André, Ann Charlotte, Bob, Kåre Ronny';

$actors = array();

foreach (explode(',', $item->casting) as $actor) {
    $actor = trim($actor);
    $actors[] = ''. $actor .'';
}

echo implode(', \n', $actors);

Which would produce the following output:

André,
Ann Charlotte,
Bob,
Kåre Ronny

I kind of prefer building the $actors array, as that allows me easily to change output variants. But you could also opt for using echo directly, which would eliminate the need for the entire $actors table. I did however removing the original $actors and included that directly in the foreach, as I try to keep intermediary variables to a minimum.

PS! Depending on font settings and encoding elsewhere, you could consider calling htmlspecialchars() in addition before using in the URL.


Object oriented python I am starting my adventure with Python and object oriented programming.

I wrote a script for salting and hashing password. My main goal is practice with OOP, not cryptography.
I know there is no error handling.

Wonder how improve my code it and make more object oriented.
It would be nice if someone more experienced can take a look and give me some advice how to make it better.
Any help for newbie would be great!

Description:

The script is not really useful in real life.

1st part:
Hash and salt password and next encode to base64 with added salt (needed for future comparing hashes). Algorithm: base64( SHA256(password + salt) + salt)
Salt is X random bytes where X can be specify but by default it is 16.

2nd part (some kind of authorization):
Compare input hash (e.x. from database) with new created hash from input plain password.
New salt is created from  salt taken from input hash.

Code:

import base64
import hashlib
import os

class Hashing(object):

    # base64( SHA256(password + salt) + salt)

    # generate new salt (default 16 bytes)
    def generate_new_salt(self, salt_ln=16):

        self.new_salt = os.urandom(salt_ln)

        print(f'new salt: {self.new_salt}')

        return self.new_salt

    # get salt from hash
    def get_old_salt(self, input_hash, salt_ln=16):

        self.old_salt = base64.b64decode(input_hash)[-salt_ln:]

        print(f'old salt: {self.old_salt}')

        return self.old_salt

    # compute hash using parameters
    def compute_hash(self, password, salt):

        self.salt = salt
        self.enc_password = password.encode()

        # hashing SHA256(password + salt)
        hash_object = hashlib.sha256(self.enc_password + salt)

        # add salt to hash and encode to base64
        hash_b64 = base64.b64encode(hash_object.digest() + salt)

        print(f'new_hash: {hash_b64}')
        return hash_b64

    # create hash from new or old salt
    def create_hash(self, password, salt_ln=16,old_salt=None):

        if old_salt:    #if old salt then use it
            self.salt = old_salt
        else:           #else generate new salt
            self.salt = Hashing().generate_new_salt(salt_ln)

        hash = Hashing().compute_hash(password, self.salt)

        return hash

    # compare input hash with created using salt get from input
    def compare_hashes(self, password, old_hash, salt_ln=16):

        self.enc_password = password.encode()

        #get salt from input hash
        self.old_salt = Hashing().get_old_salt(old_hash, salt_ln)

        #recreat input hash
        re_hash = Hashing().create_hash(password,salt_ln, self.old_salt)

        print(f'Compare: old_hash: {old_hash}')
        print(f'Compare: new_hash: {re_hash}')

        #compare
        if re_hash.decode() == old_hash.decode():
            return True
        else:
            return False

#code below is just for testing

NewSalt = Hashing().generate_new_salt()

Hash = Hashing().create_hash('pass')

OldSalt = Hashing().get_old_salt(Hash)

CompareHash = Hashing().compare_hashes('pass', Hash)

if CompareHash:
    print('HASHES THE SAME')
else:
    print('NOT THE SAME')

print(CompareHash)

Questions:

What is a real difference between these two? With and w/o self.X works

def generate_new_salt(self, salt_ln=16):
        self.new_salt = os.urandom(salt_ln)
        print(f'new salt: {self.new_salt}')
        return self.new_salt

def generate_new_salt(self, salt_ln=16):
    new_salt = os.urandom(salt_ln)
    print(f'new salt: {new_salt}')
    return new_salt

I have problem with passing default value of salt length, salt_ln = 16. For me it does not look nice when it is replicated in every method. Is there any way to do it more global?

Questions

self is the instance of whatever class the method is defined on.
In your example the method is generate_new_salt which is defined on the Hashing class.

When you use my_hashing = Hashing() you're creating an instance and binding it to the variable my_hashing. When you then call the method, via something like my_hashing.generate_new_salt(...) then self is the same as my_hashing. They both point to the same instance of Hashing.

>>> class Foo:
        def comp(self):
            return my_foo == self

>>> my_foo = Foo()
>>> my_foo.comp()
True

This means that self.foo = ... binds ... to the instance. This allows you to then use self.foo to get the value, ..., back at a later date. Without having to pass it as an argument. If you have no reason to use self.foo rather than just foo, then using self.foo is not a good idea.
This comes from two different but linked things.

You're trying to be too helpful. A lot of your code is not doing much, and is just calling other functions.

This is a common problem when programming, and so I would recommend you follow the KISS principle. If you don't need the helper now, then you don't need it at all.
You're not really using objects to their potential. If you wrap all of your methods in staticmethod decorators then nothing will change. This highlights that you're not really following OOP. And it highlights you're barely using the features class provides.

Given the above points, it would be best to start over.

Review

It's normally recommended to not use print. There are exceptions, but in most cases you're better served either by just not having print or using logging.

There is one exception, which is to interact with the user via the CLI. And in this case Hashing does not need to do that. Where you're testing needs it and so you should only have them below that comment "code below is just for testing".
Inheriting from object, Hashing(object), is redundant in Python 3.
Rather than using an if to compare something and then return True and False. You can just return the comparison.

return re_hash.decode() == old_hash.decode()

It would be best if the code below the comment "code below is just for testing" were in a function. This would mean that the global namespace isn't polluted with things that don't need to be there.
It's best to use an if __name__ == '__main__': guard. Since you only run the main function if you're testing, this will allow you to import the code without the testing code running.

From the ground up

Merging both the hash and the salt into the encoding, "base64(hash + salt)", is a smart way to keep the hash and the salt together. However it would be clearer if you create a class that stores the hash and the salt independently.

class PasswordHash:
    def __init__(self, hash, salt):
        self.hash = hash
        self.salt = salt

ph = PasswordHash(b'hash', b'salt')
print(ph.hash)  # b'hash'
print(ph.salt)  # b'salt'

From here you should be able to see two things:

The method get_old_salt is now obsolete.
You no longer need to care about the salt length.

When you first define a class, after writing the __init__, you should ask yourself if the class will ever be printed. If it could be printed, then you should make at least one of these dunder methods, double underscore method.

__repr__ - Instructions on how to rebuild the method.
__str__ - A human friendly interpretation of the class.

In this case __repr__ could make sense, but __str__ doesn't.

def __repr__(self):
    return f'PasswordHash({self.hash}, {self.salt})'

You should be able to see, __repr__ outputs the exact text we used to make the instance.

>>> print(PasswordHash(b'hash', b'salt'))
PasswordHash(b'hash', b'salt')

From here we need to think about how we should hash the password, since we don't want to store the password in plaintext. And so we can add compute_hash to PasswordHash, with 2 modifications.

I prefer the name hash_password, as it explains what it does, and what it does it on.
Secondly I would make it a staticmethod. This comes with a couple of reasons why:

It allows us to create PasswordHash from a password easier.
It allows for subclasses of PasswordHash to change the way the class hashes things.

@staticmethod
def hash_password(password, salt):
    hash = hashlib.sha256(password.encode() + salt)
    return base64.b64encode(hash.digest())

Currently making a PasswordHash from a password is pretty ugly.

ph = PasswordHash(PasswordHash.hash_password(password, salt), salt)

Since the current interface for making a PasswordHash from a password is not the nicest. And is the standard way to use the class, we can make a class method to make it pretty.

The reason this is a classmethod and not a standard method is because we need it to run before __init__. It is also a classmethod and not a staticmethod as we'll be using the first argument, cls, to instantiate the class.

@classmethod
def from_password(cls, password, salt):
    return cls(cls.hash_password(password, salt), salt)

Now making the PasswordHash from a password is really clean.

ph = PasswordHash.from_password('pass', salt)

We're almost at the end. The last thing to do is to add the spiritual child of compare_hashes to the class. Which is really simple since we have self.salt and PasswordHash.hash_password.

def compare(self, password):
    return self.hash == self.hash_password(password, self.salt)

Tying this all together you can get:

import base64
import hashlib
import os

def new_salt(length=16):
    return os.urandom(length)

class PasswordHash:
    def __init__(self, hash, salt):
        self.hash = hash
        self.salt = salt

    @classmethod
    def from_password(cls, password, salt):
        return cls(cls.hash_password(password, salt), salt)

    def __repr__(self):
        return f'PasswordHash({self.hash}, {self.salt})'

    @staticmethod
    def hash_password(password, salt):
        hash = hashlib.sha256(password.encode() + salt)
        return base64.b64encode(hash.digest())

    def compare(self, password):
        return self.hash == self.hash_password(password, self.salt)

def main():
    salt = new_salt()
    print('salt:', salt)
    hash = PasswordHash.from_password('pass', salt)
    print('PasswordHash:', hash)
    print('hash:', hash.hash)
    print('salt:', hash.salt)
    print('compare pass:', hash.compare('pass'))
    print('compare bar :', hash.compare('bar'))

if __name__ == '__main__':
    main()

To make the comparison to be more OOP you could rename compare to the dunder method __eq__. And then you could use it via the == operator.

print('compare pass:', hash == 'pass')
print('compare bar :', hash == 'bar')


Calculate date for ordinal day of week in month I am writing some date utility methods for a project.
One of the methods will answer the following questions (for example):

'When is the (3rd) (Monday) of (February) in the year (2018)'
'When is the (1st) (Wednesday) of (October) in the year (2020)'

The method accepts the 4 int value parameters as arguments, and will return a Date.

Here is the method as written:

/**
 * Example usage:
 * find the 3rd Monday of February, in the year 2018 getDayOfMonth(3, Calendar.MONDAY, Calendar.FEBRUARY, 2018)
 * find the 1st Tuesday of October, in the year 2017 getDayOfMOnth(1, Calendar.TUESDAY, Calendar.OCTOBER, 2017)
 * @param n occurrence count
 * @param dayOfWeek day of week to find
 * @param month month to use
 * @param year year to use
 * @return Date that the nth dayOfWeek occurs
 */
public static Date getDate(int n, int dayOfWeek, int month, int year){
    Calendar calendar = Calendar.getInstance();
    calendar.set(Calendar.YEAR, year);
    calendar.set(Calendar.MONTH, month);
    calendar.set(Calendar.DAY_OF_MONTH, 1);

    int matchCount = 0;

    while(true){
        if(calendar.get(Calendar.DAY_OF_WEEK) == dayOfWeek) {
            matchCount++;
        }
        if(matchCount == n){
            break;
        }
        calendar.add(Calendar.DATE, 1);
        if(calendar.get(Calendar.MONTH) != month){
            break;
        }
    }

    if(matchCount != n){
        throw new RuntimeException("error");
    }

    return calendar.getTime();
}

I am struggling to come up with a good name for the method and the 1st parameter.

For the parameter, may I suggest nthOccurrence? Alternatively, anything that communicates what exactly it means should be alright, given that it's at a pretty contained location - even nthTimeDayOfWeekOccursInMonth wouldn't be terrible, especially since dayOfWeek and month are also parameters.

The method is harder, mainly because the thing you're calculating doesn't have a great name. "Return the nth occurrence of a weekday in a given month of a given year" doesn't lend itself to brevity. I would take one of two routes:

Does the assignment / class / professor have a term for this functionality? "Weekday oracle" or something equally obscure works perfectly if it's the way that everyone else who will see this code knows to refer to this thing.

If not, I would name it what it is; something like getDateOfNthTimeDayOfWeekOccursInMonth - this would pair well with naming that first parameter nthOccurrence. In this case, I think information (what exactly can I expect this to do) trumps brevity, because what it does is not very common / intuitive.


Index IValueConverter Optimisation I've created an IValueConverter in order to Bind to the index value of a Collection.
However it seems to be rather slow/infefficient and I wonder if there is a better/different way to do this.
public class IndexValueConverter : IValueConverter
{
    public object Convert(object value, Type targetType, object parameter, CultureInfo culture)
    {
        try
        {
            CollectionView collectionView = (CollectionView) parameter;
            IList collection = (IList) collectionView.ItemsSource;

            int convertedValue = collection.IndexOf(value) + 1;
            return convertedValue;
        }
        catch (Exception e)
        {
            Debug.WriteLine(e);
            return -1;
        }
    }

    public object ConvertBack(object value, Type targetType, object parameter, CultureInfo culture)
    {
        throw new NotImplementedException();
    }
}

XAML
<CollectionView
    x:Name="JobPins"
    ItemsSource="{Binding ACollectionView}">
    <CollectionView.ItemTemplate>
        <DataTemplate x:DataType="ADataModel">
            <Label
                BackgroundColor="Black"
                Style="{StaticResource ListLabels}"
                Text="{Binding .,
                Converter={StaticResource IndexConverter},
                ConverterParameter={x:Reference Name=JobPins}}"
                TextColor="WhiteSmoke" />
        </DataTemplate>
    </CollectionView.ItemTemplate>
</CollectionView>

The best way to optimize that is store indexes directly in the source data. So, the int public Property in the ADataModel. It's rather faster to refill indexes there with O(n) complexity than with O(n2) using Converter.
The code can be a bit optimized: it's better to use bindings to pass the data to Converter. But it's not possible to pass a data to ConverterParameter using Binding. The MultiBinding+IMultiValueConverter is the solution.
<CollectionView ItemsSource="{Binding ACollectionView}">
    <CollectionView.ItemTemplate>
        <DataTemplate x:DataType="{x:Type ADataModel}">
            <Label BackgroundColor="Black" Style="{StaticResource ListLabels}" TextColor="WhiteSmoke" >
                <Label.Text>
                    <MultiBinding Converter="{StaticResource IndexConverter}">
                        <Binding Path="."/>
                        <Binding Path="ItemsSource" RelativeSource="{RelativeSource AncestorType=CollectionView}"/>
                    </MultiBinding>
                </Label.Text>
            </Label>
        </DataTemplate>
    </CollectionView.ItemTemplate>
</CollectionView>

Alternative Binding to the Colletion as an option.
<Binding Path="DataContext.ACollectionView" RelativeSource="{RelativeSource AncestorType=CollectionView}"/>

Avoid giving names to controls where possible, it would give more flexibility for the future UI improvements.
public class IndexValueConverter : IMultiValueConverter
{
    public object Convert(object[] values, Type targetType, object parameter, CultureInfo culture)
    {
        return values[1] is IList list ? list.IndexOf(values[0]) + 1 : -1;
    }

    public object[] ConvertBack(object value, Type[] targetTypes, object parameter, CultureInfo culture)
    {
        throw new NotSupportedException("Converting index of the List back is not supported");
    }
}

Avoid throwing Exception in converters if you don't want to crash the app. Don't build the app's logic on Exceptions in general.


Parse python code, for specific pep8 issues I am aware, that libraries exist for parsing python code, however, for the sake of learning how they parse errors, I'm creating a script that checks a file for only 6 Pep8 errors just for reference.
This is how my current 6 Pep8 functions look (they append an issue to the issues list if the issue was found)
"""
[S001] Line is longer than 79 characters
[S002] Indentation is not a multiple of four
[S003] Unnecessary semicolon after a statement (note, semicolons are admissible in comments)
[S004] At least two spaces before inline comments required
[S005] TODO found (only in comments; the case does not matter)
[S006] More than two blank lines used before this line (must be output for the first non-empty line)
"""
def S001(self, ln_num: int, line: str):
    if len(line) > 79:
        self.issues.append(f"Line {ln_num}: S001 Too Long")

def S002(self, ln_num: int, line: str):
    indentation_length = len(line) - len(line.lstrip())
    if indentation_length % 4 != 0:
        self.issues.append(f"Line {ln_num}: S002 Indentation is not a multiple of four")

def S003(self, ln_num: int, line: str):
    regex1 = re.compile("(.*)((;(\s)*#)|(;$))")
    regex2 = re.compile("#.*;")
    if regex1.search(line) and not regex2.search(line):
        self.issues.append(f"Line {ln_num}: S003 Unnecessary semicolon")

def S004(self, ln_num: int, line: str):
    regex = re.compile("(([^ ]{2})|(\s[^ ])|([^ ]\s))#")
    if regex.search(line):
        self.issues.append(f"Line {ln_num}: S004 At least two spaces before inline comments required")

def S005(self, ln_num: int, line: str):
    regex =  re.compile("#(.*)todo", flags=re.IGNORECASE)
    if regex.search(line):
        self.issues.append(f"Line {ln_num}: S005 TODO found")

def S006(self, ln_num: int, line: str):
    if self.code[ln_num-4:ln_num-1] == ['', '', ''] and line != "":
        self.issues.append(f"Line {ln_num}: S006 More than two blank lines used before this line")

Testcases:
""" Test case 1 """
print('What\'s your name?') # reading an input
name = input();
print(f'Hello, {name}');  # here is an obvious comment: this prints greeting with a name

very_big_number = 11_000_000_000_000_000_000_000_000_000_000_000_000_000_000_000
print(very_big_number)

def some_fun():
    print('NO TODO HERE;;')
    pass; # Todo something
""" END """

""" Test Case 2 """
print('hello')
print('hello');
print('hello');;;
print('hello');  # hello
# hello hello hello;
greeting = 'hello;'
print('hello')  # ;
""" END """

""" Test Case 3 """
    print('hello')
    print('hello')  # TODO
    print('hello')  # TODO # TODO
    # todo
    # TODO just do it
    print('todo')
    print('TODO TODO')
    todo()
    todo = 'todo'
""" END """

""" Test Case 4 """
print("hello")

print("bye")

print("check")
""" END """

""" Test Case 5 """
print('hello!')
# just a comment
print('hello!')  #
print('hello!')  # hello

print('hello!') # hello
print('hello!')# hello
""" END """

Testcase 1 Expected Output:
Line 1: S004 At least two spaces before inline comment required
Line 2: S003 Unnecessary semicolon
Line 3: S001 Too long
Line 3: S003 Unnecessary semicolon
Line 6: S001 Too long
Line 11: S006 More than two blank lines used before this line
Line 13: S003 Unnecessary semicolon
Line 13: S004 At least two spaces before inline comment required
Line 13: S005 TODO found

I am aware my code, is not optimal and doesn't satisfy every edge case, but I want an idea, on how they parse the errors properly. I would like improvements or better ideas on how to parse for errors since I personally don't like my answers.

When I copy&paste your code into my editor, it immediately greets me with 50 errors and warnings. To be fair, some of these are duplicates, because I have multiple linters configured. However, well over 20 of those are real. And ironically, at least 6 of those would have been reported by your own code!
PEP8 violations

Maximum line length
Blank lines (after the module docstring before the first method)
Single space around operators
Function names should be lower_snake_case

Missing import
You are missing the import for re.
Missing definitions
You are missing the definitions for issues and code.
Invalid escape sequence
\ is the escape character in string literals, therefore \s is interpreted as an escape sequence, but it is not a valid escape sequence. If you want to have a literal backslash in your string, you need to escape it with another backslash: \\s.
Missing docstrings
All of your functions are missing docstrings.
Naming
regex, regex1, and regex2 are not very descriptive names.
Bugs
S001 should use 72 as the maximum line length for docstrings and comments, and only use 79 for code. Also, there is a problem with PEP8 itself: it limits the line length to 79/72 characters, but it should instead limit the line length to 79/72 columns. There are characters that take up 2 columns, and there are characters that take up 0 columns. Your code uses characters, as specified by PEP8, so it is correct as far as PEP8 is concerned, but columns would make more sense.
S003 will incorrectly flag this:
"""
;
"""

S004 will incorrectly flag this:
" #"

S005 will incorrectly flag this:
"# TODO"


Get Ajax request implementation without jQuery I'm writing a pixel library to do an AJAX request so I'm not allowed to use any helper.

function ajax(options) {
  var xhr = new XMLHttpRequest();
  var opts = Object.assign({
    withCredentials: false,
    method: 'GET'
  }, options);

  xhr.withCredentials = options.withCredentials;
  xhr.open(opts.method, opts.url);
  xhr.setRequestHeader('Accept', 'text/plain');
  xhr.send(null);
  return {
    done: function(cb) {
      xhr.onreadystatechange = function onStateChange() {
        if (this.readyState === 4) {
          if (this.status >= 200 && this.status < 300) {
            cb(this.responseText);
          } else {
            cb('error');
          }
        }
      };
      xhr.ontimeout = function(e) {
        console.error(e);
        cb(false);
      };

      xhr.onerror = function(e) {
        console.error(e);
        cb(false);
      };
    }
  }
};

ajax({
  url: '/echo/json'
}).done(function(response) {
  console.log(response);
})

I also have my code in a jsfiddle in case anyone wants to test the code.

Note: This library doesn't support POST as I want to keep the code as less as possible. It will only support GET.

Your ajax() function gives me a good first impression. The structure seems sound and the event handling well done - the done() event handler is called exactly once (someone else might want to confirm, though).

A few suggestions:

var opts = Object.assign({...}, options);

opts vs. options is inconsistent and carries no semantics. Reassigning to options would mutate the argument which could cause issues for the user. jQuery names the merged options and defaults settings, which might be a little bit more clear.
if (this.status >= 200 && this.status < 300) {

This line seems good for now. You might want to check 304 in a future version when allowing users to supply caching headers. However, such requests don't respond with data and are probably not within the intended use of your library right now.
xhr.send(null);

The send method accepts an optional parameter which is ignored and set to null for GET requests. Therefore, I would recommend the simpler xhr.send().
if (this.readyState === 4) {

Add more semantic by replacing 4 with XMLHttpRequest.DONE - not available in old browsers such as IE 8.
cb('error');

This seems like a design flaw to me. A user cannot distinguish between an 'error' string received from the server vs. supplied as a result of an invalid HTTP status code. You already supply false on multiple occasions - when a network level error or timeout occurs - which is easily distinguished from a valid response by type checking. Why not supply false in the above case, too? If that's not an option, supply an error status flag or object or introduce error callback(s).


Simple GUI using tabs to separate application windows I'm designing a simple GUI using tabs to separate each 'window' of the application. There are 3 main tabs, each with 2 sub-tabs.

In one class I create the entire hierarchy of panes and panels:

mainPane = new JTabbedPane();
mainPane.setBounds(0, 0, WIDTH, HEIGHT);

//instantiate main tabs, get panes added to them
createTab = new JPanel();
createTab.setLayout(null);

customizeTab = new JPanel();
customizeTab.setLayout(null);

viewTab = new JPanel();
viewTab.setLayout(null);

//instantiate main panes, get sub-tabs added to them
createPane = new JTabbedPane();
createPane.setBounds(0, 0, WIDTH, HEIGHT);

customizePane = new JTabbedPane();
customizePane.setBounds(0, 0, WIDTH, HEIGHT);

viewPane = new JTabbedPane();
viewPane.setBounds(0, 0, WIDTH, HEIGHT);

//instantiate sub-tabs, get elements added to them
createEntityTab = new JPanel();
createEntityTab.setLayout(null);

createDealTab = new JPanel();
createDealTab.setLayout(null);

customizeEntityTab = new JPanel();
customizeEntityTab.setLayout(null);

customizeDealTab = new JPanel();
customizeDealTab.setLayout(null);

viewEntityTab = new JPanel();
viewEntityTab.setLayout(null);

viewDealTab = new JPanel();
viewDealTab.setLayout(null);

//add sub-tabs to main panes
createPane.addTab("Create Entity",null, createEntityTab);
createPane.addTab("Create Deal", null, createDealTab);

customizePane.addTab("Customize Entity", null, customizeEntityTab);
customizePane.addTab("Customize Deal", null, customizeDealTab);

viewPane.addTab("View Entity", null, viewEntityTab);
viewPane.addTab("View Deal", null, viewDealTab);

//add main panes to main tabs
createTab.add(createPane);
customizeTab.add(customizePane);
viewTab.add(viewPane);

//add main tabs to mane pane
mainPane.addTab("Create", null, createTab, "");
mainPane.addTab("Customize", null, customizeTab, "");
mainPane.addTab("View", null, viewTab, "");

and now to add elements to each sub-tab, I would like to do something like this:

new CreateEntity(createEntityTab);
new CreateDeal(createDealTab);

i.e., passing the "sub-tab" and then adding elements in the CreateEntity class.

This seems to work, but my question is the following:

Is using a constructor to pass a JPanel, and then adding elements in the new class considered bad coding?

I know using null in layout is bad practice. It's just for the question.

Is using a constructor to pass a JPanel, and then adding elements in the new class considered bad coding?

Not necessarily. However, if you only create an instance of the other class just to do things in the constructor, then you don't need the new class - and in that case it is considered bad coding.

If that's all you do in the constructor, then move the code from the constructor to a method (which you can place more or less anywhere, but I'd recommend having it in the original class unless you feel that the original class gets too big).

private void createEntityTab(JPanel panel) {
   ...
   panel.add(...);
   ...
}

Actually, it might be even better to let this method return the JPanel, so that you can use this code:

createPane.addTab("Create Entity", null, createEntityTab());

And use the method:

private JPanel createEntityTab() {
   JPanel panel = new JPanel();
   panel.setLayout(null);
   ...
   panel.add(...);
   ...
   return panel;
}


Loops through each row in a binary image and gets the width of each group of black or white pixels Example image:

Code:

#get height and width of image
height, width = possible_barcode_img.shape[:2]

#prepare list for rows
barcode_rows = []

for i in range(height):

    # set variables and list for beginning of row
    white_bar_width = 0
    black_bar_width = 0
    barcode_row = []

    for j in range(width):
        if possible_barcode_img[i,j] == 0:
            #add to count
            black_bar_width = black_bar_width +1

            #ensure last bar is gotten if there was one
            if white_bar_width > 0:
                #add bar to row
                barcode_row.append(["white", white_bar_width])
                white_bar_width = 0

        elif possible_barcode_img[i,j] > 0:
            #add to width
            white_bar_width = white_bar_width +1

            #ensure last bar is gotten if there was one
            if black_bar_width > 0:
                #add bar to row
                barcode_row.append(["black", black_bar_width])
                black_bar_width = 0

    #ensure last bar is gotten since a row just finished
    if white_bar_width > 0:
        #add bar to row
        barcode_row.append(["white", white_bar_width])

    elif black_bar_width > 0:
        #add bar to row
        barcode_row.append(["black", black_bar_width])

    #add entire row to rows
    barcode_rows.append(barcode_row)

Print row 50 of the picture in the question print(barcode_rows[50]):

[['black', 3], ['white', 7], ['black', 4], ['white', 6], ['black', 4], ['white', 6], ['black', 13], ['white', 5], ['black', 4], ['white', 15], ['black', 4], ['white', 5], ['black', 5], ['white', 5], ['black', 13], ['white', 15], ['black', 13], ['white', 5], ['black', 13], ['white', 6], ['black', 3], ['white', 15], ['black', 5], ['white', 4], ['black', 4], ['white', 14], ['black', 13], ['white', 6], ['black', 4], ['white', 14], ['black', 13], ['white', 14], ['black', 5], ['white', 5], ['black', 4], ['white', 5], ['black', 4], ['white', 14], ['black', 4], ['white', 5], ['black', 4], ['white', 5], ['black', 13], ['white', 14], ['black', 13], ['white', 5], ['black', 14], ['white', 4], ['black', 5], ['white', 4]]

50 bars found as in the picture, ready for decoding.

This has quite an effect on fps when doing it with video. I know that I don't have to loop through the entire image, I will have it send to a decoder after each row of pixels and end the loop if successful.

However I'm hoping there's a more optimized way to do this than using the Python loops on each individual pixel because I don't think the above will be fast enough for my needs.

You don't need a number of pixels, you need to detect narrow bars and wide bars. Use first black and white bar width as a pattern with 15% margin for next bars. The wide bar has 2.0 to 3.0 times the width of a narrow bar. Try to detect start or stop code at the begin of selected line, if it is correct, then read the rest of the line...
After every 5 black+white bars (in the right direction) you can decode two digits, black bars define 1st and white 2nd ...
Bars have weights: 1st: 1, 2nd: 2, 3th: 4, 4th: 7: 5th 0. Add weights of every wide bars, and you get the digit (11 = 0).
If 3 lines give you the same result, this is probably your code...

EDIT:

You can see the algorithm on your code: