Given sum and product, find the two variables Basically, find x and y given the product = xy and sum = x + y. I would like review on optimization, clean code, and complexity.  It looks like O(1), but the while loop makes me wonder if it's something different.

/**
 *  x will always be less than or equal to y.
 */
final class Variables {
    private final int x;
    private final int y;

    Variables(int x, int y) {
        this.x = x;
        this.y = y;
    }

    public int getX() {
        return x;
    }

    public int getY() {
        return y;
    }

    @Override public String toString()  {
        return " x = " + x + " y = " + y;
    }
}

/**
 * @author SERVICE-NOW\ameya.patil
 */
public final class SimultaneousEquations {

    private SimultaneousEquations () {}

    /**
     * Provided we dont have positive product and -ve sum,
     * return value would be the smallest posive integer of the variable.
     *
     * @param product : product of x and y
     * @param sum     : sum of x and y
     * @return        : smallest positive integer of x and y.
     */
    private static int getX(int product, int sum) {
        assert product != 0;
        assert product < 0 || (product > 0 && sum > 0);

        int x = 1;
        while (x*x != (sum * x  - product)) {
            x++;
        }
        return x;
    }

    /**
     * Given a product and a sum, return two variables.
     *
     * @param sum       : sum of two variables
     * @param product   : product of two variables
     */
    public static Variables getVariables(int product, int sum) {
        if (product == 0) {
            if (sum < 0) {
                return new Variables(sum, 0);
            } else {
                return new Variables(0, sum);
            }
        }

        if (product < 0) {
            int x = getX(product, sum);
            return new Variables(product/x, x);
        }

        if (sum < 0) {
            int x = -getX(product, -sum);
            return new Variables(product/x, x);
        }

        int x = getX(product, sum);
        return new Variables(x, product/x);
    }

    public static void main(String[] args) {
        // 0 10
        Variables v1 = SimultaneousEquations.getVariables(0, 10);
        boolean result1 = v1.getX() == 0 && v1.getY() == 10;
        System.out.println(result1);

        // 2 5
        Variables v2 = SimultaneousEquations.getVariables(10, 7);
        boolean result2 = v2.getX() == 2 && v2.getY() == 5;
        System.out.println(result2);

        // -2 5
        Variables v3 = SimultaneousEquations.getVariables(-10, 3);
        boolean result3 = v3.getX() == -2 && v3.getY() == 5;
        System.out.println(result3);

        // 2 -5
        Variables v4 = SimultaneousEquations.getVariables(-10, -3);
        boolean result4 = v4.getX() == -5 && v4.getY() == 2;
        System.out.println(result4);

        // -2 -5
        Variables v5 = SimultaneousEquations.getVariables(10, -7);
        boolean result5 = v5.getX() == -5 && v5.getY() == -2;
        System.out.println(result5);
    }
}

Before we start guessing, lets solve the equations properly. We can substitute the equations so that we get product = x · (sum - x) = -x² + sum·x ⇔ 0 = x² - sum·x + product, with y = sum - x.

The equation can be solved by x = sum/2 ± √(sum²/4 - product) (see Reduced Quadratic Equation). Note that we get zero, one or two  distinct solutions. For example the special case of product = 0 leads to x = sum/2 ± sum/2 ⇒ {x = 0, x = sum}. It is up to you if you pick a specific solution or output both. We have no solutions in ℝ when the expression inside the square root is negative. However, there would still be complex solutions.

Restricting yourself to integers only isn't valid, and will miss some solutions. Consider product = 1, sum = 4. This is solved by x=3.73205, y=0.267949.

Note that as this equation can be solved arithmetically, there is no need for guessing via loops.

Because you resort to guessing, your code contains various bugs, e.g. it will fail to handle parameters with no solution like sum = 4, product = 5. Instead, you will go into an unterminated loop.

When unit-testing your code, you should be using a proper test harness instead of  some test cases in a main. You might like JUnit. If you want to thoroughly test a method, do not only test a few simple cases:

Explicitly test failure cases, e.g. parameters with no solution.
Test especially large inputs to see if you correctly guarded against overflows (you didn't).
Test random input (and record the seed!). It is easy to verify the correctness of the solution. This fuzzy testing may hit cases you didn't consider.
Cover inputs around zero especially well, e.g. vary each parameter from -2 to 2 by 0.5-increments (after verifying manually that each parameter combination actually has a solution). Although this wouldn't hit another bug in your code, the combination 0, 0 is suspiciously absent. The combination 1, 0 would have exposed the bug that you don't handle inputs without solutions.


Pulling log messages from a MySQL table I want to get this code reviewed for a function I wrote that pulls log messages from a MySQL table:

def get_logs_messages(request_id, destination=None):
    """
        get logs from deployment table
    :param int request_id: request id of selected deployment.
    :param str destination: destination site
    :return: list of messagesfor selected deployment based on request id.
    :rtype: list
    """
    conn = MySQLdb.connect(**dbconfig)
    query = "SELECT deployment_id " \
            "from deployment " \
            "WHERE request_id={request_id} " \
            " {dest}".format(
                request_id=request_id,
                dest="AND DESTINATION = '{loc}'".format(loc=destination) if destination else "")
    cursor = conn.cursor()
    cursor.execute(query)
    data = cursor.fetchall()
    cursor.close()
    msgs = []
    for item in data:
        query = "SELECT modified_dt , message from scripts_deployment_logs WHERE deployment_id = {}".format(item[0])
        cursor = conn.cursor()
        cursor.execute(query)
        result = cursor.fetchall()
        for dt, msg in result:
            msgs.append("{dt}: {msg}".format(dt=str(dt), msg=msg))
        cursor.close()
    conn.close()
    return msgs

I get a request ID from a web interface which is using a REST API to call this function, and this function retrieves the deployment id based on which it displays the logs. I am not very fluent with the MySQL part so I have used mysql.connector with simple SQL queries.

Instead of joining the data of two tables in Python, you should une an SQL join query to ask the database do the work for you.

You also should not inject parameters directly into the query and let the driver do it for you by means of parametrized queries.

Lastly, a list-comprehension is better suited to build your output than using append.

Proposed improvements:

def get_logs_messages(request_id, destination=None):
    parameters = (request_id,)
    query = (
        'SELECT logs.modified_dt, logs.message '
        'FROM scripts_deployment_logs AS logs '
        'LEFT JOIN deployment ON logs.deployment_id = deployment.deployment_id '
        'WHERE deployment.request_id = %s'
    )

    if destination is not None:
        query += ' AND deployment.destination = %s'
        parameters = (request_id, destination)

    conn = MySQLdb.connect(**dbconfig)
    cursor = conn.cursor()
    cursor.execute(query, parameters)
    messages = [
        '{}: {}'.format(dt, message)
        for dt, message in cursor.fetchall()
    ]
    cursor.close()
    return messages


Prettify this code - combine items in a list Can anyone help me prettify this code? The idea is to combine adjacent time spans so

[0:00-1:00, 1:00-2:00, 3:00-4:00] becomes [0:00-2:00, 3:00-4:00]

private static IEnumerable<Span> CombineAdjacentSpans(IEnumerable<Span> spans)
{
    var combined = new List<Span>();
    var list = spans.OrderBy(s => s.Start).ToList();

    var candidate = list.FirstOrDefault();
    foreach (var span in list.Skip(1))
    {
        if (span.Start == candidate.End)
        {
            candidate.End = span.End;
            continue;
        }

        combined.Add(candidate);
        candidate = span;
    }

    if (candidate != null)
        combined.Add(candidate);

    return combined;
}

You can shorten this and get lazy evalution using yield:

private static IEnumerable<Span> CombineAdjacentSpans(this IEnumerable<Span> spans)
{
    Span current = null;
    foreach (var span in spans.OrderBy(s => s.Start)) {
        if (current == null) { current == span; }
        // I made this handle overlaps, but if you don't care about those
        // you can change the comparison to just ==
        else if (span.Start <= current.End && span.End > current.End) {
            // current.End = span.End
            // it's probably better to make a new span rather than modifying the
            // current one here since that way this method doesn't modify elements
            // in the original list. If span is a struct, then modification would be
            // fine but we'd have to rely on a bool rather than checking against null
            current = new Span { Start = current.Start, End = span.End };
        } else {
            yield return current;
            current = span;
        }
    }

    if (current != null) { yield return current; }
}

This way, something like mySpans.CombineAdjacentSpans().Take(3) will only do the work of combining the first 3 spans (although OrderBy will still have to look at all elements in the input sequence).

In response to Chris's comment, if you really want the last if statement inside the loop here's one way I can think of to do it:

private static IEnumerable<Span> CombineAdjacentSpans(this IEnumerable<Span> spans)
{
    Span current = null;
    // could also use Enumerable.Repeat() instead of new[] { ... }
    foreach (var span in spans.OrderBy(s => s.Start).Concat(new[] { default(Span) })) {
        if (current == null) { current == span; }
        // I made this handle overlaps, but if you don't care about those
        // you can change the comparison to just ==
        else if (span.Start <= current.End && span.End > current.End) {
            // current.End = span.End
            // it's probably better to make a new span rather than modifying the
            // current one here since that way this method doesn't modify elements
            // in the original list. If span is a struct, then modification would be
            // fine but we'd have to rely on a bool rather than checking against null
            current = new Span { Start = current.Start, End = span.End };
        } else {
            yield return current;
            current = span;
        }
    }
}


Jquery Selector optimization for multiple clicks I have multiple inputs that each one has an increment and decrement button

The buttons will be pressed multiple times so i am looking to optimize my selector:

So should i keep the first one (which selects by attribute) or the second? Or anything more optimized.

$("[data-id-col="+thisID+"]")

$("input:tel#"+thisID+")

$("body").on('click','.incButton',e=>{
    let thisID = e.currentTarget.getAttribute("d-id");
    let colInput = $("[data-id-col="+thisID+"]");
    colInput.val( parseInt( colInput.val())+1);
    colInput.change();
});

Html:

Input

<div class="handle-counter" id="handleCounter3">
    <input type="tel" data-id-col="8900" id="qty_inputCol" class="qty_col">
</div>

Button

<button type="button" d-id="8888" class="btn btn-primary btn-block inc"></button>

https://learn.jquery.com/performance/optimize-selectors/

Your second selector doesn't work with the data provided input:tel#8888 is not the id= of the input.

IDs should start with a letter (for compatibility), so if you changed your input to <input id="inp8888"> then you could use input:tel#inp8888 in which case, you can use $("#inp8888") as long as your IDs are unique (which they should be).

Selecting by ID is the fastest selection method.

  jQuery uses document.getElementById(), which is faster

https://learn.jquery.com/performance/optimize-selectors/#id-based-selectors


Using getters and setters to change another variable C# Is it a bad practice to use getters and setters this way?

Below is my code:

public struct Settings
{
    public static int workMinutes
    {
        get
        {
            return Properties.Settings.Default.workMinutes;
        }
        set
        {
            Properties.Settings.Default.workMinutes = value;
            save();
        }
    }

    public static int restMinutes
    {
        get
        {
            return Properties.Settings.Default.restMinutes;
        }
        set
        {
            Properties.Settings.Default.restMinutes = value;
            save();
        }
    }

    public static void init()
    {
        if(Properties.Settings.Default.workMinutes == 0)
            Properties.Settings.Default.workMinutes = 5;

        if(Properties.Settings.Default.restMinutes == 0)
            Properties.Settings.Default.restMinutes = 5;

        save();
    }

    private static void save()
    {
        Properties.Settings.Default.Save();
    }
}

I don't think it's a bad practice at all. What you did is called encapsulation. It enables you to exchange the underlying configuration provider (currently the Properties.Settings) to something else without breaking your application because it doesn't rely directly on it.

It's also called the Dependency Inversion Principle the higher module (your application) relies on an abstraction (your Settings) and not on the low level module (Properties.Settings). Usually it's build upon abstract types (or interfaces) but a lite version of this could be your settings layer.


Keep database lookup in memory and get values in frequent Is there a way to write it in more efficient and modern (Up to date design pattern) way?

 static Dictionary<string, Dictionary<string, string>> m_LUApplicationParams;
    public string LUApplicationParams(int applicationId, string GuidId, string variable)
            {
                if (m_LUApplicationParams == null)
                {
                    var blLU = new LU();
                    DataTable dt = new DataTable();
                    dt = blLU.GetLUApplicationVaribales(applicationId);

                    m_LUApplicationParams = new `Dictionary<string,Dictionary<string,string>>();`

                    for (int i = 0; i < dt.Rows.Count; i++)
                    {

                        if (!m_LUApplicationParams.ContainsKey(dt.Rows[i]["LangId"].ToString()))
                        {
                            m_LUApplicationParams.Add(dt.Rows[i]["LangId"].ToString(), new Dictionary<string, string>());
                        }

                        m_LUApplicationParams[dt.Rows[i]["LangId"].ToString()].Add(dt.Rows[i]["Variable"].ToString(), dt.Rows[i]["Content"].ToString());                                    }

                }

                return m_LUApplicationParams[GuidId].Where(y => y.Key == variable).FirstOrDefault().Value.ToString();
            }

Some quick notes:

LUApplicationParams is not a good method name. It doesn't tell me what it does, and moreover it isn't even a verb.
m_LUApplicationParams isn't a good name for a field or property. I'm not a fan of the "m_" naming, it contains an obscure abbreviation or acronym (LU), and it doesn't tell me what it contains, which is especially bad considering this is a Dictionary<string, Dictionary<string, string>>.
I'd move all of the logic inside if (m_LUApplicationParams == null) to a separate method. I even wonder whether this would be the right place for such logic (although I have done such things myself).
blLU doesn't tell me anything, even when it is used like var blLU = new LU();.
Why do you do DataTable dt = new DataTable();? It's pointless since the next line assigns a value to dt anyway.
"LangId" is used three times. Suppose you ever need to change it: wouldn't it be handier if it were a constant?
dt.Rows[i]["LangId"].ToString() is used three times: then why not assign it to a variable?
GuidId is a parameter and thus should be CamelCase. But is GuidId even a proper name, considering that the other code suggests it is actually an id to identify the langhuage: "LangId"?
What does variable represent? Even the field name isn't helping, since that is "Variable" also. I'd hate to be the next person to maintain this code since obviously a lot of information is missing from it.
You do .FirstOrDefault() and then immediately use the .Value of the result; but what if there is no value?
Also, .FirstOrDefault() suggests multiple values are possible, is that a possibility you want? And why would the first result be the correct one?
Why do you do .ToString() to the result of .FirstOrDefault().Value? Why do you fear that the value of a Dictionary<string, string> key-value pair wouldn't be a string?
You do .Where and then .FirstOrDefault: replace this by .FirstOrDefault(y => y.Key == variable).

If you need to fetch a specific value from a DataRow, why not extract such code to a method?

private string GetValue(DataRow row, string key)
{
    return row[key].ToString();
}

Avoid doing Dictionary.ContainsKey: if you need to use the value of said key, use Dictionary<TKey, TValue>.TryGetValue.

var row = dt.Rows[i];
var languageId = GetValue(row, "LangId");

Dictionary<string,string>() contentByVariable;
if(!m_LUApplicationParams.TryGetValue(languageId, out contentByVariable))
{
    contentByVariable = new Dictionary<string,string>();
}

contentByVariable.Add(GetValue(row, "Variable"), GetValue(row, "Content"));
m_LUApplicationParams[languageId] = contentByVariable;

But if I were you I'd rethink this entire logic. A Dictionary<string, Dictionary<string, string>> is ugly and unwieldy; IMHO the nested dictionary should be a custom class.

I also feel this code is odd and possible buggy: data is retrieved for an applicationId yet this parameter is completely absent in m_LUApplicationParams. The GuidId that is passed is assumed to be present as a key in m_LUApplicationParams, same for the variable.


Simple File Manager with recursion in C# Simple class to simpify runtime file loading of ressources. The class reads all files from a given directory including all subdirectories and stores the name together with the path in a simple struct. If a file needs to be used by the program just pass the name to GetPath() and the class returns the location of the file.

Any suggestions/improvements about my implementation of this? Im especially not sure if making this static is the right choice.

using System.IO;
using System.Collections.Generic;

namespace Sample
{
    struct File
    {
        public string Name;
        public string Path;
    }

    static class FileManager
    {
        private static List<File> Files = new List<File>();

        public static void AddFiles(string directory)
        {
            foreach(string file in  Directory.GetFiles(directory))
            {
                Files.Add(new File() { Name = Path.GetFileName(file), Path = directory });
            }

            foreach(string subdirectory in Directory.GetDirectories(directory))
            {
                AddFiles(subdirectory);
            }
        }

        public static string GetPath(string filename)
        {
            var File = Files.Find(x => x.Name == filename);

            return File.Path;
        }

        public static void ClearFiles()
        {
            Files.Clear();
        }
    }
}

I don't think I would make this class static, unless it's going to be used extensively throughout the program. If not you can build up a large list of files, that may hang around in memory for no use, unless you remember to call ClearFiles(). Instead you could make a static method that could return an initialized object like:

public static FileManager Create(string directoryPath)
{
  FileManager fm = new FileManager();
  fm.AddFiles(directoryPath);
  return fm;
}

If you have a need for it, then make this instance as static somewhere in the application.

public static string GetPath(string filename)
{
  var File = Files.Find(x => x.Name == filename);
  return File.Path;
}

It returns only a first match of possible more matches, which will be in a directory high in the hierarchy, but what if you actually seek a path to a file in a subdirectory?

I think I would return a list/array/IEnumerable instead and let the client filter as needed.

Besides that, file names are case insensitive, so you should do:

Files.Find(x => string.Equals(x.Name, filename, StringComparison.CurrentCultureIgnoreCase));

public static void AddFiles(string directory)
{
  foreach (string file in Directory.GetFiles(directory))
  {
    Files.Add(new File() { Name = Path.GetFileName(file), Path = directory });
  }
  foreach (string subdirectory in Directory.GetDirectories(directory))
  {
    AddFiles(subdirectory);
  }
}

Nice recursive method. As an alternative you could consider to use DirectoryInfo instead - it can handle the recursive search for you:

  DirectoryInfo directory = new DirectoryInfo(directoryPath);
  Files.AddRange(
    directory
    .GetFiles("*.*", SearchOption.AllDirectories)
    .Select(fi => new File { Name = fi.Name, Path = fi.DirectoryName }));

There is no way to iterate through all the found File objects because the Files static member is private. I would consider to provide a public IEnumerable of some kind.

All in all, my implementation would look something like:

  public struct File
  {
    public string Name;
    public string Path;

    public override string ToString()
    {
      return $"{Name} => {Path}";
    }
  }

  public class FileManager : IEnumerable<File>
  {
    private List<File> Files = new List<File>();

    public void AddFiles(string directoryPath)
    {
      DirectoryInfo directory = new DirectoryInfo(directoryPath);
      Files.AddRange(
        directory
        .GetFiles("*.*", SearchOption.AllDirectories)
        .Select(fi => new File { Name = fi.Name, Path = fi.DirectoryName }));
    }

    public IEnumerable<string> GetPaths(string filename)
    {
      return Files
        .Where(x => string.Equals(x.Name, filename, StringComparison.CurrentCultureIgnoreCase))
        .Select(f => f.Path);
    }

    public void Clear()
    {
      Files.Clear();
    }

    public IEnumerator<File> GetEnumerator()
    {
      return Files.GetEnumerator();
    }

    IEnumerator IEnumerable.GetEnumerator()
    {
      return GetEnumerator();
    }

    public static FileManager Create(string directoryPath)
    {
      FileManager fm = new FileManager();
      fm.AddFiles(directoryPath);
      return fm;
    }
  }


Comparison of Fibonacci (Multinacci) Functions in Python3 After coming up with a formula on this Stack Overflow question, defeating recursion limits and finding a few new ways of calculating these numbers (as per this Stack Overflow question), I decided that it was time to write up the solutions and submit them for a proper review.

For those that are not familiar with Fibonacci and more generally, Multinacci numbers, here are the rules:

The numbers correspond to the number of pairs of rabbits at year n
Rabbits take 1 year to mature
Rabbits reproduce after they mature, to produce 1 (or k in the case of Multinacci) pairs of rabbits
There is 1 pair at year 1, and none at year 0

from functools import lru_cache

limit = 100

# Classic recursive
def fibnum1(n, k=1):
    """Returns the multinacci number of order k at generation n"""
    if n <= 0:
        return 0
    if n == 1 or n == 2:
        return 1
    # build up the cache as needed to defeat recursion bounds
    global limit
    if n > limit:
        for temp_n in range(limit, n, 100):
            _fibnum(temp_n, k)
        limit = n + 100
    return _fibnum(n, k)

# Recursive with matrix multiplication
def fibnum2(n, k=1):
    """Returns the multinacci number of order k at generation n"""
    if n <= 0:
        return 0
    if n == 1 or n == 2:
        return 1
    return _matrix_fib(n, k)[1]

# Iterative
def fibnum3(n, k=1):
    """Returns the multinacci number of order k at generation n"""
    if n <= 0:
        return 0
    fibnums = [0, 1, 1]
    for i in range(3, n+1):
        fibnums.append(fibnums[i-1] + k * fibnums[i-2])
    return fibnums[n]

# Helper for fibnum1, memoized of course
@lru_cache(maxsize=None)
def _fibnum(n, k=1):
    if n <= 0:
        return 0
    if n == 1 or n == 2:
        return 1
    return _fibnum(n-1, k) + k * _fibnum(n-2, k)

# Helper for fibnum2, memoized of course
@lru_cache(maxsize=None)
def _matrix_fib(n, k):
    if n == 1:
        return [0, 1]
    else:
        f = _matrix_fib(n // 2, k)
        c = k * f[0] * f[0] + f[1] * f[1]
        d = f[1] * (f[1] + 2 * k * f[0])
        if n % 2 == 0:
            return [c, d]
        else:
            return [d, k * c + d]

As you can see, there are three distinct functions here for calculating these numbers: fibnum1, fibnum2, and fibnum3. They are nearly functionally identical, as all 3 are super fast (memoization for the win!) and work for relatively big numbers quite well. fibnum2 will fail eventually (dumb recursion limit...), but it is at too large of a number to be practically reached. I would like a comparative review, along with any other comments on any aspect of the code.

Everything looks fine to me. Just some small suggestion:

Don't define limit as a global. You can make it an attribute of the function itself: fibnum1.limit

In the matrix version I would return a tuple instead of a list. Also I would define two variables for f[0] and f[1] as follows:

@lru_cache(maxsize=None)
def _matrix_fib(n, k):
    if n == 1:
        return (0, 1)
    else:
        f0, f1 = _matrix_fib(n // 2, k)
        c = k * f0 * f0 + f1 * f1
        d = f1 * (f1 + 2 * k * f0)
        if n % 2 == 0:
            return (c, d)
        else:
            return (d, k * c + d)


Cordova file management I am making an application using Phonegap where users can select and share files. When the user selects it, even if it goes offline or closes the app, I should upload it back to the server, so I need to store the reference file path.

Steps I have used:

User selects/captures video or image using Camera.
File may not contain extension to name in Android every time and in iOS when selected from Gallery.
Get mimeType and then copy the file to myApp's folder with a new name and extension.
Then again get mimeType and return.

Is it correct to copy a file to another location whenever the user selects a media file?

 var obj = {
    destinationType: navigator.camera.DestinationType[fromCamera ? 'FILE_URI' : 'NATIVE_URI'],
    // If I use NATIVE_URI for when taking from Camera it returns null.
    sourceType: navigator.camera.PictureSourceType[fromCamera ? 'CAMERA' : 'PHOTOLIBRARY'],
    mediaType: navigator.camera.MediaType[image ? 'PICTURE' : 'VIDEO']
}, that = this;

if (image && fromCamera) {
    obj.correctOrientation = true;
    obj.quality = 40;
}

return new Promise(function (resolve, reject) {
    navigator.camera.getPicture(function (url) {
        var fail = function () {
            resolve(false);
        }
        window.setTimeout(function () {
            // mimeType uses window.resolveLocalFileSystemURL to get fileEntry
            that.mimeType(url).then(function (mimeObj) {
                /*
                    mimeObj contains object like below
                    {lastModified:'', fileType:'', fileExtension:'', name : ''}
                */
                if (mimeObj) {
                    // that.fileSystem() resolves fileSystem
                    that.fileSystem().then(function (fs) {
                        // dirName is where I want to store all myApp's media files.
                        var dirName = 'myApp/media/' + (image ? "images" : "videos") + '/';
                        that.getFile(url).then(function (fileEntry) {
                            // getFile uses window.resolveLocalFileSystemURL and returns fileEntry
                            that.moveTo(fs, fileEntry, dirName, mimeObj.name).then(function (entry) {
                                // MoveTo fill copy the same file to directory where I am manintaing files.
                                mimeObj.localURL = entry.nativeURL;
                                /*
                                    mimeObj is failing to find extension
                                    when user selects it from different albums. But not when user uses Camera.
                                    So I am making again mimeType call.
                                */
                                that.mimeType(mimeObj.localURL).then(function (mimeObj) {
                                    // This gives me exact mimeObj that I need.
                                    resolve(mimeObj);
                                });
                            }, fail);
                        }, fail);
                    }, fail);
                } else {
                    resolve(false);
                }
            });
        }, 0);

    }, function (fail_message) {
        window.setTimeout(function () {
            reject(fail_message);
        }, 0);
    }, obj);
});

Interesting question,

it is of course not good to fill up memory with copies of pictures. Especially with this scandal in recent memory where deleted pictures haunt users.

It seems from reading your code that you take a copy of the file whether or not you have the extension and MIME type. At least only copy if you see no other way.

Furthermore, from the documentation ( and I could be wrong ), the FileEntry object has a method called file which creates a File Object which has a type property which is the mime type of the file. Could this solve your conundrum ? If that does not work, you should see if you can file a bug.

From a real code review perspective, this is my quick once over:

Respect for dealing with this immature API
JsHint has nothing on this, except for 1 missing semicolon
Have you considered using 'use strict' ?
The multi-line comments definitely stop the reading flow, but removing them would reduce the value of your commenting. I wonder if those multi-liners can be part a narrative on top of your function..
You are at the limit of having to break things up into helper functions, as per this arrow half:

                            });
                        }, fail);
                    }, fail);
                }, fail);
            } else {
                resolve(false);
            }
        });
    }, 0);

}, function (fail_message) {


Web application for recording blog comments I am working on some coursework for a simple Java EE web application that records blog comments. On the add comment page there are two fields: name and comments

I am using a RequestDispatcher to pass control to an acknowledgement.jsp if the fields are not empty, and an error.jsp if one (or both) are empty passing a relevant message to the acknowledgement page.

This is the logic I have:

  RequestDispatcher disp;
  if (!name.isEmpty() && !comment.isEmpty())
  {
      CommentBean commentBean = new CommentBean();
      commentBean.setComment(comment);
      commentBean.setName(name);
      req.setAttribute("comment", commentBean);

      disp = req.getRequestDispatcher("acknowledgement.jsp");
      disp.forward(req, resp);

      return;
  }
  else if (!name.isEmpty() && comment.isEmpty())
  {
      disp = req.getRequestDispatcher("error.jsp");
      statusMessage = "Comment field empty";
      req.setAttribute("status", statusMessage);
      disp.forward(req, resp);
  }
  else if (name.isEmpty() && !comment.isEmpty())
  {
      disp = req.getRequestDispatcher("error.jsp");
      statusMessage = "Name field empty";
      req.setAttribute("status", statusMessage);
      disp.forward(req, resp);
  }
  else
  {
      disp = req.getRequestDispatcher("error.jsp");
      statusMessage = "Name and comment fields empty";
      req.setAttribute("status", statusMessage);
      disp.forward(req, resp);
  }

The code does what I want but I can't help thinking this could be written a lot better although I am not sure how to go about it.

That's easy: You're repeating quite a few things, so don't do it.

disp.forward(req, resp);

This is the easiest, copied in each of the 4 branches, but needn't be. Drop the needless return and move it after the branches.

But in general, deduplication is hard and it's much better to learn to avoid duplications. Just imagine Ctrl-C and Ctrl-V are broken and you must type using one finger only. :D You could write a method accepting statusMessage and handling the problem. But it's simple enough so you can do it inline.

The basic idea is "compute the statusMessage, leave it empty in case there's no problem and handle it":

if (comment.isEmpty()) {
    statusMessage = name.isEmpty()
        ? "Name and comment fields empty"
        : "Comment field empty";
} else {
    statusMessage = name.isEmpty()
        ? "Name field empty"
        : "";
}

if (statusMessage.isEmpty()) {
    CommentBean commentBean = new CommentBean();
    commentBean.setComment(comment);
    commentBean.setName(name);
    req.setAttribute("comment", commentBean);
    disp = req.getRequestDispatcher("acknowledgement.jsp");
} else {
    disp = req.getRequestDispatcher("error.jsp");
}

disp.forward(req, resp);


Solver for Jumble puzzle Here is my first attempt to solve Jumble puzzle:

import argparse
from itertools import permutations

parser = argparse.ArgumentParser(description='Solver for Jumble')
parser.add_argument('jumbledwords', nargs='+',
                    help='One or more jumbled words')
argv = parser.parse_args()

# http://www-01.sil.org/linguistics/wordlists/english/wordlist/wordsEn.txt
words = [line.rstrip() for line in open('wordsEn.txt')]
for jumbledword in argv.jumbledwords:
    perms = set([''.join(p) for p in permutations(jumbledword)])
    legalwords = [word for word in perms if word in words]
    # print(len(perms))
    print(jumbledword, legalwords)

Any suggestions for improvement?

Close what you open

words = [line.rstrip() for line in open('wordsEn.txt')]

Here you opened a file but you did not close it! In fact I suggest to use with that will handle closing automagically.

with open('wordsEn.txt') as f:
    words = [line.rstrip() for line in f.read()]

Let the user change the file

You should let the user input as an optional arg his own filename, I may have my own wordlist that is not called exactly: 'wordsEn.txt', the dafault file should be None (You read directly from the webpage).

set built-in operations

If you change the below line to be:

words = set([line.rstrip() for line in f.read().splitlines()])

You can then use set intersection:

legalwords = perms & words

To enhance both clarity and performance.

Allowing long words

The complexity of finding all the permutations of a word is O(N!): for a mere 20 characters word that means 2.432 * 10**18 combinations, I suggest comparing the sorted words for better efficiency if you need to run the script with long words.


Human-readable number of bytes in Haskell This is a tiny etude on a theme given here, using Python: Provide a function that generates a human readable file size such as 10.0GiB, 13B, 42.2TiB from a given integer number. (A similar Ruby etude is here.) The function should

gracefully handle any input (even negative numbers, although they don't make  sense for counting bytes - maybe they could represent negative offsets)
print numbers smaller than 1024 without a decimal point
print all other numbers with one decimal place and the appropriate unit (KiB, MiB, etc.)

It's more verbose than the python counterpart, so the question is whether this can be made more terse.

    humanReadableBytes :: Integer -> String
    humanReadableBytes size | null pairs = printf "%.0fZiB" (size'/1024^7)
                            | otherwise  = if unit=="" then printf "%dB" size
                                           else printf "%.1f%sB" n unit
    where
        (n, unit) = head pairs
        pairs = dropWhile ((1024<).abs.fst)
             (zip (map ((size'/).(1024^)) [0..]) units) :: [(Double, String)]
        size' = fromIntegral size
        units = ["","Ki","Mi","Gi","Ti","Pi","Ei","Zi"]

Right away it can be noted that the use of String is suboptimal, but Text.Printf does not allow Text, so printf ought to be replaced.

We begin with the "loop". map ((size'/).(1024^)) [0..] is iterate (/1024) size. This has not necessarily the same effect, since (x/y)/y is not the same as x/y² when using floating point arithmetic, but it's fine for our cause. It also works like the Python code, which was your original intention.

We now have

humanReadableBytes :: Integer -> String
humanReadableBytes size | null pairs = printf "%.0fZiB" (size'/1024^7)
                        | otherwise  = if unit=="" then printf "%dB" size
                                       else printf "%.1f%sB" n unit
where
    (n, unit) = head pairs
    pairs = zip (iterate (/1024) 'size)) units
    size' = fromIntegral size :: Double
    units = ["","Ki","Mi","Gi","Ti","Pi","Ei","Zi"]

And that's all we can improve in terms of terseness at that point without changing your code fundamentally. The main difference between the Python and the Haskell implementation is the mutation of the original num and the early exit from the function.

I would recommend to check the "B" case earlier by the way:

humanReadableBytes :: Integer -> String
humanReadableBytes size
  | abs size < 1024 = printf "%dB" size
  | null pairs      = printf "%.0fZiB" (size'/1024^7)
  | otherwise       = printf "%.1f%sB" n unit
where
  ...


Simple Expression Calculator in C The calculator evaluates a mathematical expression with binary operators +, -, *, / and ^ in C. I store numbers and operators into two different arrays and process them according to precedence.

Please review my code and suggest me on changes on approaches to make it scalable to add unary operators and parenthesis.

#include <stdio.h>
#include <math.h>

double evaluate (char []);
int precedence(char);
double calculate(char , double , double );
char checknumber(char);

int main(void)
{
    char expression[100];
    double result;

    printf("Enter the Expression: "); scanf("%[^\n]", expression);
    result = evaluate(expression);
    printf("Result = %lf\n", result);
    return 0;
}

double evaluate(char expr[])
{
    double numbers[5]; int nsi = 0;
    char operators[5]; int osi = 0;
    char numbuf[16]; int nbi = 0;
    char ch; int  i = 0;

    while ((ch = expr[i]) != 0) {
        if (checknumber(ch))
        {
            numbuf[nbi++] = ch;
            if (!checknumber(expr[i + 1]))
            {
                numbuf[nbi] = 0; nbi = 0;
                sscanf(numbuf, "%lf", &numbers[nsi++]);
            }
        }
        else
        {
            while ((osi > 0) && (precedence(ch) <= precedence(operators[osi - 1])))
            {
                numbers[nsi - 2] = calculate(operators[osi - 1], numbers[nsi - 2], numbers[nsi - 1]);
                osi--; nsi--;
            }
            operators[osi++] = ch;
        }
        i++;
    }
    while (osi > 0) {
        numbers[nsi - 2] = calculate(operators[osi - 1], numbers[nsi - 2], numbers[nsi - 1]);
        osi--; nsi--;
    }
    return numbers[0];
}

char checknumber(char ch)
{
    if ((ch >= '0' && ch <= '9') || ch == '.') return 1; else return 0;
}
int precedence(char ch)
{
    int precedence;
    switch (ch)
    {
    case '+':
    case '-':
        precedence = 0;
        break;
    case '*':
    case '/':
        precedence = 1;
        break;
    case '^':
        precedence = 2;
    }
    return precedence;
}

double calculate(char moperator, double num1, double num2)
{
    double result;
    switch (moperator)
    {
    case '+':
        result = num1 + num2;
        break;
    case '-':
        result = num1 - num2;
        break;
    case '*':
        result = num1 * num2;
        break;
    case '/':
        result = num1 / num2;
        break;
    case '^':
        result = pow(num1, num2);
        break;
    default:
        printf("Invalid Operator\n");
        exit(-1);
    }
    return result;
}

In calculate why not just return directly from the different cases? This would be simpler than setting a temporary value, breaking from the switch and returning that value.

switch (moperator)
{
case '+':
    return num1 + num2;
case '-':
    return num1 - num2;
case '*':
    return num1 * num2;
case '/':
    return num1 / num2;
case '^':
    return pow(num1, num2);
default:
    printf("Invalid Operator\n");
    exit(-1);
}


jQuery code for displaying server status from XML feed I am using the below jQuery and HTML to read in and use an XML server status feed for a server widget. This is the code:

XML Feed:

<status>

Here is a proposition.
The idea is to define an object that maps the property (http, ftp...) with related images url indexed by possible property values (OK, WARNING,...).

With this approach :

get the img src for a service : object[service_name][service_status]
get the service status : $(xml).find(service_name).text()
get the img dom element $("." + service_name + " img")

Code :

    $(document).ready(function() {
       var config = {
            "http":{
                "OK":"http://placehold.it/40/00cc33/ffffff",
                "WARNING":"http://placehold.it/40/ff6600/000000",
                "CRITICAL":"http://placehold.it/40/ff0000/000000",
                "UNKNOWN":"/path/to/unknow"
            },
            "ftp":{
                "OK":"http://placehold.it/40/00cc33/ffffff",
                "WARNING":"http://placehold.it/40/ff6600/000000",
                "CRITICAL":"http://placehold.it/40/ff0000/000000",
                "UNKNOWN":"/path/to/unknow"
            },
            "mysql":{
                "OK":"http://placehold.it/40/00cc33/ffffff",
                "WARNING":"http://placehold.it/40/ff6600/000000",
                "CRITICAL":"http://placehold.it/40/ff0000/000000",
                "UNKNOWN":"/path/to/unknow"
            },
            "load":{
                "OK":"http://placehold.it/40/00cc33/ffffff",
                "WARNING":"http://placehold.it/40/ff6600/000000",
                "CRITICAL":"http://placehold.it/40/ff0000/000000",
                "UNKNOWN":"/path/to/unknow"
            }
        };

       function refreshView (xml) {
            for (var name in config) {
                $("." + name +" img").attr("src",config[name][$(xml).find(name).text()] );
            }
       }

        $.ajax({
            type:  'GET',
            url:   'https://status.clook.net/xml/status/harvey.xml',
            dataType: 'xml',
            success: refreshView
        });
    });

You can simplify if status images are the same for all services.

var img = {
  "OK":"http://placehold.it/40/00cc33/ffffff",
  "WARNING":"http://placehold.it/40/ff6600/000000",
  "CRITICAL":"http://placehold.it/40/ff0000/000000",
  "UNKNOWN":"/path/to/unknow"
}
var services = ['http','ftp','mysql']

function refreshView (xml) {
  for (var i=0; i<services.length; i++) {
    $("." + services[i] +" img").attr("src",img[ $(xml).find(services[i]).text()] );
  }
}


My first graph traversal code I'm trying to solve a graph traversal problem I found, and was wondering how I could improve my implementation. Currently it seems a little convoluted and long.

The problem is as follows:
I have a matrix of size (rows x cols). This matrix has some cells that are empty (designated by a 0) and some cells that are blocked off (designated by a 1). Given a length, I would like to find a consecutive set of empty cells (ie, a path) of size length.

For this specific code instance, I actually hardcoded the matrix, as well as the length. My approach was:

to loop through the matrix;
at each position try to find a path, if that position was empty and not yet visited;
if so, then find all adjacent positions that fit the same condition (ie, empty and not visited);
if adjacent positions exist, continue finding further adjacent positions until my path is complete in size;
if a path is not possible, then print "impossible".

My code is as follows:

def positionInGraph(pos):
    return ((pos[0] >= 0) and (pos[1] >= 0)) and ((pos[0] < rows) and (pos[1] < cols))

def positionNotVisited(pos):
    return not(pos in visited)

def positionIsEmpty(pos):
    return (graph[pos[0]][pos[1]]==0)

def getAdjacent(posx, posy):
    adjQueue = [(posx-1,posy), (posx, posy+1), (posx+1, posy), (posx, posy-1)]
    for i in adjQueue:
        if (positionInGraph(i) and positionNotVisited(i) and positionIsEmpty(i)):
            yield i

def getOnePath(pos):
    counter = 0
    pathList = []
    processQueue = []
    if (not(positionIsEmpty(pos)) or (pos in visited)):
        return (pathList, counter)
    else:
        processQueue.append(pos)
    while (processQueue and (counter < length)):
        currentPos = processQueue.pop()
        visited.add(currentPos)
        pathList.append(currentPos)
        counter = counter + 1
        adjQueue = getAdjacent(currentPos[0],currentPos[1])
        if (not(adjQueue) and counter < length):
            pathList.pop(currentPos)
            counter -= 1
        for i in adjQueue:
            processQueue.append(i)

    print pathList
    return (pathList, counter)

def findFinalPath():
    #5 4 8
    # oxoo
    # xoxo
    # ooxo
    # xooo
    # oxox

    for i in xrange(0,rows):
        for j in xrange(0,cols):
            path = getOnePath((i,j))
            if path[1] == length:
                return path[0]

    return "impossible"

visited = set()
graph = [[0,1,0,0],[1,0,1,0],[0,0,1,0],[1,0,0,0],[0,1,0,1]]

rows = 5
cols = 4
length = 8

findFinalPath()

1. Bug

I tried a different test case:

graph = [[1, 0, 0, 0, 1], [1, 1, 0, 1, 0], [0, 0, 0, 0, 0], [1, 1, 0, 1, 1], [1, 0, 0, 0, 0]]
rows = 5
cols = 5
length = 8

and then findFinalPath returned the following path:

[(0, 1), (0, 2), (1, 2), (2, 2), (2, 1), (2, 0), (3, 2), (4, 2)]

This is not a legal path, since (2, 0) is not adjacent to (3, 2).

I give an explanation for this bug in 2.18 below, but it would be a good exercise to for you to try to figure out for yourself what's going wrong here. How does your search manage to step from (2, 0) to (3, 2)?

2. Comments on your code

No docstrings! What do your functions do, and how do you call them?
This kind of program is an excellent candidate for doctests.
The use of global variables makes your code difficult to re-use. For example, the global variable graph means that it would be awkward if you ever wanted to find paths in two different graphs.

When you have persistent state (like your graph) with associated operations (like your function positionInGraph) it's often a good idea to organize your code into classes and methods respectively. See section 3 below for how I would do this in your case.
You don't guard the execution of the test program with if __name__ == __main__:. This means that when I import your program into an interactive Python interpreter, it immediately starts running, which makes it harder for me to test. Put your test code into test functions or doctests.
Returning the string "impossible" when no path is found isn't the best way to design the interface. Returning an exceptional value to indicate failure tends to be error-prone: it's too easy for the caller to forget to check. It's usually better to raise an exception when exceptional circumstances are encountered.
You represent your coordinates as a pair (y, x). It would be slightly easier to understand the output of your program if you represented coordinates in the usual way (x, y). This would of course require a corresponding change, either looking up cells with graph[pos[1]][pos[0]] (preferred) or transposing your matrix so that it is in column-major order.
You set the number of rows and columns by hand but these numbers are easy to determine: they are len(graph) and len(graph[0]) respectively. (I prefer to call these numbers height and width myself.)
Your functions positionInGraph, positionNotVisited and positionIsEmpty all take a pos as their argument, but getAdjacent takes two arguments posx and posy (which are wrongly named: posx is the y-coordinate and vice versa). You should generally strive to be consistent in little details like this: it makes it easier to remember how to call functions.
The Python style guide (PEP8) says that "Function names should be lowercase, with words separated by underscores as necessary to improve readability" and the same for variable names. So you should consider changing positionIsEmpty to position_is_empty and so on. (You're not obliged to follow PEP8 but it makes it easier for other Python programmers to read your code.)
You can use itertools.product to avoid nested loops.
The loop

for i in adjQueue:
    processQueue.append(i)

can be written

processQueue.extend(adjQueue)

You have a variable called processQueue but it is not a queue. A queue is a data structure where you add elements to one end of a list and remove them from the other end (in Python you would use collections.deque). A data structure where you add and remove elements at the same end is called a stack.
You have a variable counter that you increment and decrement in parallel with adding and removing positions to pathList, so that counter is always the length of pathList. It would be better to drop counter and call len(pathList) instead: one less thing to go wrong. (If you were worried that len(pathList) might be an O(n) operation as it is in some languages, you can look at the TimeComplexity page on the Python wiki for reassurance.)
If you have an algorithm that uses a stack, it's often easiest and clearest to implement it as a recursive function (you can implicitly use the function call stack instead of having to explicitly push and pop your own stack). See section 3 below for how to do this.
You give the maze once in a comment:

# oxoo
# xoxo
# ooxo
# xooo
# oxox

and then again in code:

graph = [[0,1,0,0],[1,0,1,0],[0,0,1,0],[1,0,0,0],[0,1,0,1]]

This violates the DRY principle (Don't Repeat Yourself): it would be easy for you to make a mistake when encoding your maze as a matrix of 1s and 0s. Why not get the computer to do it for you?
Similarly, it's hard for you to check the results of your program, because the path comes out as a list of coordinates, which is tedious and error-prone to check by eye. Even in the buggy example I gave in section 1 above, it would be easy to give it a quick look and miss the error. It would be better to present the results in a form that's easy for you to check. (See section 3 below for how I did this.)
Your code has much unnecessary use of parentheses. The lines

return ((pos[0] >= 0) and (pos[1] >= 0)) and ((pos[0] < rows) and (pos[1] < cols))
return not(pos in visited)
return (graph[pos[0]][pos[1]]==0)
if (not(positionIsEmpty(pos)) or (pos in visited)):
while (processQueue and (counter < length)):
return (pathList, counter)

can be written

return 0 <= pos[0] < rows and 0 <= pos[1] < cols
return pos not in visited
return graph[pos[0]][pos[1]] == 0
if not positionIsEmpty(pos) or pos in visited:
while processQueue and counter < length:
return pathList, counter

respectively. "Program as if you know the language"!
Explanation for the bug in section 1: your back-tracking implementation doesn't backtrack all your search state, so that the search state becomes inconsistent. When your search reaches a dead end, it pops the last position from pathList and decrements counter, but does not update the rest of the search state. This means that the next currentPos that gets popped from processQueue will not (in general) be adjacent to the end of the path. When you backtrack in a search, you need to backtrack all your state: in your case pathList, counter, processQueue, and visited all need to be backtracked, but you only update the first two of these, and so it goes wrong.

See the revised code for how I keep the search state consistent. Every addition to the state as the search goes forward:

path.append(p)
visited.add(p)

is matched by a corresponding deletion as the search backtracks:

visited.remove(p)
path.pop()

(There's more to say about this issue, but I've moved this to section 4 below.)

3. Revised code

from itertools import product

class Found(Exception): pass
class NotFoundError(Exception): pass

class Maze(object):
    """A rectangular matrix of cells representing a maze.

    Describe the maze to the constructor in the form of a string
    containing dots for empty cells and any other character for walls.

        >>> m = Maze('''.#..
        ...             #.#.
        ...             ..#.
        ...             #...
        ...             .#.#''')
        >>> print(m.format(m.path(8)))
        .#.o
        #.#o
        oo#o
        #ooo
        .#.#
        >>> print(m.format(m.path(10)))
        ... # doctest: +IGNORE_EXCEPTION_DETAIL
        Traceback (most recent call last):
          ...
        NotFoundError: no path of length 10

    """
    def __init__(self, maze):
        self.matrix = [[c != '.' for c in line] for line in maze.split()]
        self.height = len(self.matrix)
        self.width = len(self.matrix[0])
        assert all(len(row) == self.width for row in self.matrix)

    def in_bounds(self, p):
        """Return True if `p` is a legal position in the matrix."""
        return 0 <= p[0] < self.width and 0 <= p[1] < self.height

    def empty(self, p):
        """Return True if `p` is an empty position in the matrix."""
        return not self.matrix[p[1]][p[0]]

    def neighbours(self, p):
        """Generate legal positions adjacent to `p`."""
        for dx, dy in ((1, 0), (0, 1), (-1, 0), (0, -1)):
            q = p[0] + dx, p[1] + dy
            if self.in_bounds(q):
                yield q

    def path(self, n):
        """Find a path of length `n` in the maze, or raise NotFoundError if
        there is no such path.

        """
        path = []
        visited = set()
        def search(p):
            path.append(p)
            visited.add(p)
            if len(path) == n:
                raise Found()
            for q in self.neighbours(p):
                if self.empty(q) and q not in visited:
                    search(q)
            visited.remove(p)
            path.pop()
        try:
            for p in product(range(self.width), range(self.height)):
                if self.empty(p):
                    search(p)
        except Found:
            return path
        else:
            raise NotFoundError('no path of length {}'.format(n))

    def format(self, path=()):
        """Format the maze as a string. If optional argument path (an
        iterable) is given, highlight cells in the path.

        """
        path = set(path)
        def row(y):
            for x in range(self.width):
                p = x, y
                if not self.empty(p): yield '#'
                elif p in path: yield 'o'
                else: yield '.'
        return '\n'.join(''.join(row(y)) for y in range(self.height))

4. Extra credit: encapsulation of search state

In 2.18 above I discussed the need to update your search state properly when implementing a back-tracking search. As with any kind of data consistency problem, it's good practice to encapsulate operations that must be performed together.

An easy way to do this in Python is via a context manager. A "context manager" is just an object with __enter__ and __exit__ methods: when the context manager is the subject of a with statement, the __enter__ method is called on entry to the with statement, and the __exit__ method on exit. By putting your forward operations in the  __enter__ method and the back-tracking operations in the __exit__ method you can be sure that these pair up properly.

But an even easier way to do this is to use contextlib.contextmanager to build the context manager for you, like this:

from contextlib import contextmanager

def path(self, n):
    """Find a path of length `n` in the maze, or raise NotFoundError if
    there is no such path.

    """
    path = []
    visited = set()

    @contextmanager
    def visit(p):
        path.append(p)       # Forward: add p to path.
        visited.add(p)       # Forward: mark p as visited.
        yield                # Rest of search happens here.
        visited.remove(p)    # Backtrack: mark p not visited.
        path.pop()           # Backtrack: remove p from path.

    def search(p):
        with visit(p):
            if len(path) == n:
                raise Found()
            for q in self.neighbours(p):
                if self.empty(q) and q not in visited:
                    search(q)
    try:
        for p in product(range(self.width), range(self.height)):
            if self.empty(p):
                search(p)
    except Found:
        return path
    else:
        raise NotFoundError('no path of length {}'.format(n))


Program to replace tabs with blanks From my C university book 'The C Programming Language' by Brain W.Kernighan and Dennis M.Ritchie. Excercise 1 - 20.

  Write a program detab that replaces tabs in the input with the proper number of
      blanks to space to the next tab stop.
  Assume a fixed set of tab stops, say every
      n columns. Should n be a variable or a symbolic parameter?

#include <stdio.h>

#define TABSTOP 8
#define MAXLINE 1000

int getline(char s[], int len);

int main()
{
    char line[MAXLINE];

    while ((getline(line, MAXLINE) > 0))
    {
        int i, j, count;

        count = 0;
        for (i = 0; line[i] != '\0'; ++i)
        {
            if (line[i] == '\t')
            {
                for (j = 0; j < TABSTOP - count; ++j)
                {
                    putchar(' ');
                }

                count = 0;
            }
            else
            {
                ++count;

                if (count >= TABSTOP)
                    count = 0;

                putchar(line[i]);
            }
        }
    }

    system("pause");

    return 0;
}

int getline(char line[], int len)
{
    int c, i;

    for (i = 0; i < len - 1 && (c = getchar()) != EOF && c != '\n'; ++i)
        line[i] = c;

    if (c == '\n')
    {
        line[i] = c;
        ++i;
    }

    line[i] = '\0';

    return i;
}

My concerns are that I didn't get the question right, did they mean that every time I see a tab in the input I need to put the corresponding amount of spaces instead of tabs?

Are there any flaws or bugs in the code?

I didn't understood what is a 'symbolic parameter', did they mean symbolic constant?

By 'n' did they mean what I did with the 'TABSTOP' symbolic constant?

Headers and Names

You are using the system function that is declared in stdlib.h but you don't #include <stdlib.h>.
The name getline is already taken by many C implementations for an extension to the standard library's I/O functions. You shouldn't use it for your own function names.

Program structure
Your program basically consists of the main function. While simple it is, this is not a very modular design and in particular hostile to unit testing. I suggest you decouple the I/O logic from the tab expansion logic. Have one function read the input line-by-line and for each line it reads, make it call another function that processes the line and places the results in a separate buffer. It could look like this:
size_t
expand_tabs(int tabwidth, const char * input, char * output, size_t length);

The first function then outputs that buffer. This way you can, for example, unit test expand_tabs with an array of strings.
Don't put
system("pause");

in your programs.  I know many tutorials targeted at people using the Windows operating system suggest this but it is poor advice. If your program is done, make it exit. If this causes your terminal emulator to close the window before you wanted it to, fix the way you use the terminal emulator, not your program.
I/O Handling
Avoid character-based I/O.  It's slow and painful. Instead of your hand-rolled getline function, you could have used the standard library fgets function. From its man page:

Synopsis
#include <stdio.h>

char *fgets(char *s, int size, FILE *stream);

Description
fgets() reads in at most one less than size characters from stream and stores them into the buffer pointed to by s.  Reading stops after an EOF or a newline.  If a newline is read, it is stored into the buffer.  A terminating null byte ('\0') is stored after the last character in the buffer.
Return Value
fgets() returns s on success, and NULL on error or when end of file occurs while no characters have been read.

This is just what your getline function does but faster and cleaner.
Never use the seemingly simpler gets function.  It is broken and C11 removed it from the standard library.
While using stack-allocated buffers like
char line[MAXLINE];

is fast, the hard-coded limit is a little awkward. You actually took care to make your program process files with lines longer than MAXLINE correctly too. Congratulations for that. Another option would be to use a dynamically allocated buffer. If your implementation has it, the aforementioned getline function comes in handy, here. If you are following the book, I'm not sure if you've already learned about dynamic memory yet, though.
Style
While declaring functions as
int my_getline(char s[], int len);

is valid syntax, it is misleading because the first argument actually is a pointer and not an array.  So I prefer the (functionally equivalent)
int my_getline(char * s, int len);

syntax.
As long as you want to use MAXLINE to create an array, it needs to be a macro, unfortunately.  For TABSTOP there is no such need. Avoid macros if possible. Here, a static const int would have been perfectly fine. Note that in the expand_tabs function suggested above, I'm even passing it as a parameter so it doesn't depend on global constants. Of course, the constant has to be coded at some point in your program.


C++ "Vending Machine" task Task:

  Write a program that presents the user w/ a choice of your 5 favorite
  beverages (Coke, Water, Sprite, ... , Whatever). Then allow the user
  to choose a beverage by entering a number 1-5. Output which beverage
  they chose.

  ★ If you program uses if statements instead of a switch statement,
  modify it to use a switch statement. If instead your program uses a
  switch statement, modify it to use if/else-if statements.

  ★★ Modify the program so that if the user enters a choice other than
  1-5 then it will output "Error. choice was not valid, here is your
  money back."

#include <iostream>
using namespace std;

int main() {

int drink = 0;
string ifswitch;
string again = "y";

while( again == "y" || again == "Y")
{
    cout << "Would you like to use if or switch statements?: ";
    cin >> ifswitch;
    cout << endl;

    if(ifswitch == "switch" || ifswitch == "Switch")
    {
        cout << "Now using switch statements..." << endl << endl;

        cout << "Your choices of drinks are: " << endl;
        cout << "1 - Coke" << endl;
        cout << "2 - Sprite" << endl;
        cout << "3 - Water" << endl;
        cout << "4 - Orange Juice" << endl;
        cout << "5 - Apple Juice" << endl;

        cout << "What drink would you like (1/5): ";
        cin >> drink;
        cout << endl;

        switch(drink)
        {
            case 1 :
                cout << "You chose Coke." << endl;
                break;
            case 2 :
                cout << "You chose Sprite." << endl;
                break;
            case 3 :
                cout << "You chose Water." << endl;
                break;
            case 4 :
                cout << "You chose Orange Juice." << endl;
                break;
            case 5 :
                cout << "You chose Apple Juice." << endl;
                break;
            default :
                cout << "Error. Choice was not valid, Here is your money back.";

        }
    }
    else if(ifswitch == "if" || ifswitch == "If")
    {
        cout << "Now using if statements..." << endl << endl;

        cout << "Your choices of drinks are: " << endl;
        cout << "1 - Coke" << endl;
        cout << "2 - Sprite" << endl;
        cout << "3 - Water" << endl;
        cout << "4 - Orange Juice" << endl;
        cout << "5 - Apple Juice" << endl;

        cout << "What drink would you like (1/5): ";
        cin >> drink;
        cout << endl;

        if(drink == 1)
            cout << "You chose Coke." << endl;
        else if(drink == 2)
            cout << "You chose Sprite." << endl;
        else if(drink == 3)
            cout << "You chose Water." << endl;
        else if(drink == 4)
            cout << "You chose Orange Juice." << endl;
        else if(drink == 5)
            cout << "You chose Apple juice." << endl;
        else
        cout << "Error. Choice was not valid, Here is your money back.";

    }

    cout << "Would you like to go again? (Y/N)" << endl;
    cin >> again;
    cout << endl;

}

    cout << "Ending...";

}

Do not use namespace std;
Put your code into functions if possible. For example the Question for the user should be a single function.

void printOptions() {
     std::cout << "Your choices of drinks are: " << "\n";
     std::cout << "1 - Coke" << "\n";
     std::cout << "2 - Sprite" << "\n";
     std::cout << "3 - Water" << "\n";
     std::cout << "4 - Orange Juice" << "\n";
     std::cout << "5 - Apple Juice" << "\n";
     std::cout << "What drink would you like (1/5): ";

}

Note that i use "\n" instead of std::endl as this doesnt flush the stream.
Use a dedicated input function

int chooseBeverage() {
    int beverage;
    std::cin >> beverage;
    return beverage;
}

Now define a function that takes the choice and give the beverage.

enum choice {
    NONE = 0;
    COKE,
    SPRITE,
    WATER,
    ORANGE_JUICE,
    APPLE_JUICE
};

void serveBeverageIf(choice beverage) {
     if(beverage == COKE)
        std::cout << "You chose Coke." << "\n";
     else if(beverage == SPRITE)
        std::cout << "You chose Sprite." << "\n";
     else if(beverage == WATER)
        std::cout << "You chose Water." << "\n";
     else if(beverage == ORANGE_JUICE)
        std::cout << "You chose Orange Juice." << "\n";
     else if(beverage == APPLE_JUICE)
        std::cout << "You chose Apple juice." << "\n";
     else
        std::cout << "Error. choice was not valid, here is your money back.";
 }

 void serveBeverageSwitch(choice beverage) {
     switch(beverage)
     case COKE:
        std::cout << "You chose Coke." << "\n";
        break;
     case SPRITE:
        std::cout << "You chose Sprite." << "\n";
        break;
     case WATER:
        std::cout << "You chose Water." << "\n";
        break;
     case ORANGE_JUICE:
        std::cout << "You chose Orange Juice." << "\n";
        break;
     case APPLE_JUICE:
        std::cout << "You chose Apple juice." << "\n";
        break;
     default:
        std::cout << "Error. choice was not valid, here is your money back.";
        break;
     }
 }

What you could also do is use a third option utilizing a map.

void serveBeverageMap(choice beverage) {
map<choice, std::string> nameMap = {
    std::make_pair(COKE, "Coke"),
    std::make_pair(SPRITE, "Sprite"),
    std::make_pair(WATER, "Water"),
    std::make_pair(ORANGE_JUICE, "Orange juice"),
    std::make_pair(APPLE_JUICE, "Apple juice")
};
if(nameMap.find(beverage) == nameMap.end()) {
   std::cout << "Error. choice was not valid, here is your money back.";
} else {
    std::cout << You chose " << nameMap[beverage] << ".";
}
}


C++ Dynamic array based stack I have implemented a dynamic array based stack. Can someone point out any pitfalls or things that can be done better?

MyStack.h

#ifndef MYSTACK_H
#define MYSTACK_H

#include <iostream>
#include <stdexcept>

template<class T>
class MyStack
{

private:

    T* m_array;

    int m_count;

    int m_max_size;

    static const int m_growth_factor = 2;

    static const int m_initial_max_size = 10;

public:

    MyStack();

    inline MyStack(const MyStack<T> &rhs) { *this = rhs; }

    void operator=(const MyStack<T> &rhs);

    MyStack(int initial_max_size);

    ~MyStack();

    void push(T data);

    void pop();

    void clear();

    inline bool empty() { return m_count == 0; }

    inline T& top() { return m_array[m_count - 1]; }

    inline int size() { return m_count; }

private:

    void init();

    void increase_array_size();
};

template <class T>
MyStack<T>::MyStack() : m_count(0), m_max_size(m_initial_max_size)
{
    init();
}

template <class T>
MyStack<T>::MyStack(int initial_max_size) : m_count(0), m_max_size(initial_max_size)
{
    init();
}

template <class T>
MyStack<T>::~MyStack()
{
    delete [] m_array;
}

template <class T>
void MyStack<T>::init()
{
    m_array = new T[m_max_size];
    m_count = 0;
}

template <class T>
void MyStack<T>::increase_array_size()
{
    m_max_size = m_growth_factor * m_max_size;
    T* tmp = new T[m_max_size];

    for(int i = 0; i < m_count; i++)
        tmp[i] = m_array[i];

    delete [] m_array;

    m_array = tmp;
}

template <class T>
void MyStack<T>::push(T data)
{
    if(m_count == m_max_size)
        increase_array_size();
    m_array[m_count++] = data;
}

template <class T>
void MyStack<T>::pop()
{
    if(m_count == 0)
        throw std::underflow_error("Underflow Exception!!!");
    m_count--;

}

template <class T>
void MyStack<T>::clear()
{
    delete [] m_array;
    m_max_size = m_initial_max_size;
    init();
}

template <class T>
void MyStack<T>::operator=(const MyStack<T> &rhs)
{
    if(this != &rhs)
    {
        delete [] m_array;
        init();
        for(int i = 0; i < rhs.m_count;i++)
        {
            this->push(rhs.m_array[i]);
        }

    }
}
#endif // MYSTACK_H

main.cpp

#include <iostream>
#include "MyStack.h"

using namespace std;

int main(int argc, char* argv[])
{

    MyStack<int> stack;
    int sum = 0;

    for(int i = 0; i < 15; i++)
        stack.push(i);

    while(!(stack.empty()))
    {
        sum += stack.top();
        stack.pop();
    }

    cout << "sum: " << sum << endl;

    stack.push(10);
    stack.push(20);

    stack.top() -= 5;

    cout << "stack.top() is now " << stack.top() << endl;

    MyStack<char> stack2;

    stack2.push('t');
    stack2.push('s');
    stack2.push('e');
    stack2.push('T');

    while(!(stack2.empty()))
    {
        cout << stack2.top();
        stack2.pop();
    }
    cout << endl;

    stack2.push('A');
    stack2.push('B');
    stack2.push('C');

    stack2.clear();

    stack2.push('D');
    stack2.push('E');
    stack2.push('F');

    while(!(stack2.empty()))
    {
        cout << stack2.top();
        stack2.pop();
    }
    cout << endl;

}

Output:

sum: 105
stack.top() is now 15
Test
FED

Overall

Not bad.

Resizing.

The maths is in favor of using a refactoring size of less than 1.63 as it potentially allows for memory to be reused after several allocations.

    static const int m_growth_factor = 2;

But on the other hand all the standard libraries do use 2.

Copy and Swap Idiom

    inline MyStack(const MyStack<T> &rhs) { *this = rhs; }

You are defining the copy constructor in terms of the assignment operator. The idiomatic way to do this is the other way around and define the assignment operator in terms of the copy constructor. Its called the copy and swap idiom. It provides the strong exception guarantee (ie the assignment works or the object is unchanged). Your assignment operator does not provide this.

Pre-Allocation

template <class T>
void MyStack<T>::init()
{
    m_array = new T[m_max_size];
    m_count = 0;
}

Here you are pre-allocating an array of T. This means that T must be default constructible (i.e. have a zero parameter constructor). Also if T is expensive to create and you don't use the whole array then you may be creating these object unnecessarily at an expensive point in the code.

You can allocate memory without calling the constructor then use placement new during the push to copy the data into the memory pool.

The same problem also appears in increase_array_size()

Potential Leak.

template <class T>
void MyStack<T>::increase_array_size()
{
    // STUFF

    for(int i = 0; i < m_count; i++)
        tmp[i] = m_array[i];           // If this throws an exception.
                                       // then you are leaking `tmp` pointer.

    // STUFF
}

Potential Invalid object.

template <class T>
void MyStack<T>::increase_array_size()
{
    // STUFF

    delete [] m_array;         // If this throws (because a T destructor throws)
                               // then you leave this object in an undefined
                               // state and you leak the `tmp` pointer.

    // STUFF
}

Use the copy and swap idiom to provide strong exception guarantee. This will also solve the problems in the last two points.

Provide Move Semantics.

This interface provides the standard copy semantics.

template <class T>
void MyStack<T>::push(T data)  // Also not passing by value causes a copy here
                               // So prefer to pass by const reference.
{
    if(m_count == m_max_size)
        increase_array_size();
    m_array[m_count++] = data;
}

But in C++11 we introduced move semantics which is potentially faster than copy semantics and Veridic template also allowing you to build the object in place.

template <typename T>
void MyStack<T>::push(T const& data);    // Copy data into MyStack.
template <typename T>
void MyStack<T>::push(T&& data);         // Move data into MyStack
template <typename... Args>
void MyStack<T>::emplace(Args&&... data);// Build data into MyStack

Assume the user knows the pre-conditions

You can check the pop() operation and throw. But if the user has already checked and knows that the stack is not empty you are doing extra work. For example the user is popping all the values from the stack in a for loop check for empty() in each iteration. Then your check becomes redundant extra work (because empty() was already called).

template <class T>
void MyStack<T>::pop()
{
    if(m_count == 0)
        throw std::underflow_error("Underflow Exception!!!");
    m_count--;

}

So in C++ you usually provide unchecked interface (let the user do the checking).

If you want you can also provide a checked interface then that is fine but usually not the default.

For example look at std::vector. Provides operator[](std::size_t index) for unchecked access to the data. But also provides at(std::size_t index) for checked access to the index.

Un-needed work.

Do you really need to call delete here?

template <class T>
void MyStack<T>::clear()
{
    delete [] m_array;
    m_max_size = m_initial_max_size;
    init();
}

You pre-initialized all the values initially. So you are not reallying on constructor/destructor properties. So this function can be simplified to:

template <class T>
void MyStack<T>::clear()
{
    m_count = 0;
}

If you want to release all the resources and re-create the default versions you can manually destroy all the objects.

Potential Leak.

template <class T>
void MyStack<T>::operator=(const MyStack<T> &rhs)
{
    if(this != &rhs)
    {
        delete [] m_array;                  // You should not delete the current
                                            // state until you know the operation
                                            // has succeeded. If the operation fails
                                            // you currently can not recover your state.

        init();
        for(int i = 0; i < rhs.m_count;i++)
        {
            this->push(rhs.m_array[i]);
        }

    }
}
#endif // MYSTACK_H


Splitting a CSV file with headers I have been looking for an algorithm for splitting a file into smaller pieces, which satisfy the following requirements:

The first line in the original file is a header, this header must be carried over to the resulting files
The ability to specify the approximate size to split off, for example, I want to split a file to blocks of about 200,000 characters in size.
File must be splitted at line boundaries

More info:

This is part of my web service: the user uploads a CSV file, the web service will see this CSV is a chunk of data--it does not know of any file, just the contents.
The web service then breaks the chunk of data up into several smaller pieces, each will the header (first line of the chunk). It then schedules a task for each piece of data. I don't want to process the whole chunk of data since it might take a few minutes to process all of it. I want to process them in smaller size. This is why I need to split my data.
The final code will not deal with open file for reading nor writing. I only do that to test out the code.
The task to process smaller pieces of data will deal with CSV via csv.DictReader. It does not make sense to use the csv module to break up the original chunk of data into pieces. I have done some timing and my algorithm achieves better performance as opposed to reading/writing line-by-line just to break data into pieces.

Here is an example:

If my original file is:

Header
line1
line2
line3
line4

If I want my approximate block size of 8 characters, then the above will be splitted as followed:

File1:

Header
line1
line2

File2:

Header
line3
line4

In the example above, if I start counting from the beginning of line1 (yes, I want to exclude the header from the counting), then the first file should be:

Header
line1
li

But, since I want the splitting done at line boundary, I included the rest of line2 in File1.

Here is what I have so far. I am looking to turn this code segment into a procedure, but more importantly, I want the code to speed up a bit. Currently, it takes about 1 second to finish. In the final solution, In addition, I am going to do the following:

Change the procedure to return a generator, which returns blocks of data. I don't really need to write them into files. I think I know how to do this, but any comment or suggestion is welcome.
Since my data is in Unicode (Vietnamese text), I have to deal with .encode(), .decode(). Any hint to speed this up would be great.

with open('data.csv') as f:
    buf = f.read().decode('utf-8')

header, chunk = buf.split('\n', 1)

block_size = 200000
block_start = 0
counter = 0
while True:
    counter += 1
    filename = 'part_%03d.txt' % counter
    block_end = chunk.find('\n', block_start + block_size)
    print filename, block_start, block_end

    with open(filename, 'w') as f:
        f.write(header + '\n')
        if block_end == -1:
            f.write(chunk[block_start:].encode('utf-8'))
        else:
            f.write(chunk[block_start:block_end].encode('utf-8'))
        f.write('\n')

    block_start = block_end + 1
    if block_end == -1: break

1. Comments on your code

There's no documentation. What does your program do and how should it be used?
Your program is not split up into functions. This makes it hard to test it from the interactive interpreter.
The name data.csv seems arbitrary. Surely this should be an argument to the program? Similarly for 'part_%03d.txt' and block_size = 200000.
You read the whole of the input file into memory and then use .decode, .split and so on. It would be more efficient to read and write one line at a time, thus using no more memory than is needed to store the longest line in the input.
In Python, a file object is also an iterator that yields the lines of the file. It's often simplest to process the lines in a file using for line in file: loop or line = next(file).
When you have an infinite loop incrementing a counter, like this:

counter = 0
while True:
    counter += 1

consider using itertools.count, like this:

for counter in itertools.count(1):

(But you'll see below that it's actually more convenient here to use enumerate.)
Since your data is encoded in UTF-8, and only character you actually look for in the input is the newline character (\n), there is no need for you to decode the input or encode the output: you could work with bytes throughout. (But if you insist on decoding the input and encoding the output, then you should  pass the encoding='utf-8' keyword argument to open.)

2. Revised code

Here's how I'd implement your program. The only tricky aspect to it is that I have two different loops that iterate over the same iterator f (the first one using enumerate, the second one using itertools.chain). This works because the iterator state is stored in the object f, so the two loops can independently fetch lines from the iterator and the lines still come out in the right order.

from itertools import chain

def split_file(filename, pattern, size):
    """Split a file into multiple output files.

    The first line read from 'filename' is a header line that is copied to
    every output file. The remaining lines are split into blocks of at
    least 'size' characters and written to output files whose names
    are pattern.format(1), pattern.format(2), and so on. The last
    output file may be short.

    """
    with open(filename, 'rb') as f:
        header = next(f)
        for index, line in enumerate(f, start=1):
            with open(pattern.format(index), 'wb') as out:
                out.write(header)
                n = 0
                for line in chain([line], f):
                    out.write(line)
                    n += len(line)
                    if n >= size:
                        break

if __name__ == '__main__':
    split_file('data.csv', 'part_{0:03d}.txt', 200000)

3. Generating the output blocks instead

Do you really want to process a CSV file in chunks? I can't imagine why you would want to do that. Surely either you have to process the whole file at once, or else you can process it one line at a time?

Processing a file one line at a time is easy:

with open(filename, encoding='utf-8') as f:
    header = next(f)
    for line in f:
        # process line

But given that this is apparently a CSV file, why not use Python's built-in csv module to do whatever you need to do? Like this:

import csv
with open(filename, encoding='utf-8', newline='') as f:
    reader = csv.reader(f)
    header = next(reader)
    for row in reader:
        # row[0] is the first field in this row, and so on.


Web-scraping a list of lawyers I've written a program to get the names along with the titles of some practitioners out of a webpage. The content stored within disorganized HTML elements (at least it seemed to me) and as a result the output becomes messier. However, I've managed to get the output as it should be.

The way I've written the scraper serves its purpose just fine but the scraper itself looks ugly. Is there any way to make the scraper look good and still have it do the same job as it is doing currently?  For example, is there a way to eliminate the exception handlers?

This is how I've written it:

import requests
from bs4 import BeautifulSoup

url = "https://www.otla.com/index.cfm?&m_firstname=a&seed=566485&fs_match=s&pg=publicdirectory&diraction=SearchResults&memPageNum=1"

def get_names(link):
    res = requests.get(link)
    soup = BeautifulSoup(res.text,'lxml')
    for content in soup.select(".row-fluid"):
        try:
            name = content.select_one(".span5 b").get_text(strip=True)
        except: continue  #let go of blank elements
        else: name

        try:
            title = content.select_one(".span5 b").next_sibling.next_sibling.strip()
        except: continue  #let go of blank elements
        else: title

        print(name,title)

if __name__ == '__main__':
    get_names(url)

Don't use a bare except statement if it's not absolutely necessary. Catch the errors you anticipate and explicitly want to handle.

Also, there's no reason for the duplicated try/except/else statements, and no use, in this case, of the else statements at all. You can collapse everything into a single try/except, since you do the same thing regardless of which line throws the error.

You also have a little code duplication. Don't Repeat Yourself (DRY).

def get_names(link):
    res = requests.get(link)
    soup = BeautifulSoup(res.text,'lxml')
    for content in soup.select(".row-fluid"):
        try:
            element = content.select_one(".span5 b")
            name = element.get_text(strip=True)
            title = element.next_sibling.next_sibling.strip()
            print(name,title)
        except AttributeError:
            continue


New list from the elements of two lists popping sequentially Can this be shortened, optimised or made more pythonic ?

a = [1,9,'foo',5,7]
b = ['bar',4,8,6]
c = []

min_len = min(len(a),len(b))

for i in range(min_len):
  c.extend([a.pop(0), b.pop(0)])

c.extend(a)
c.extend(b)

print c

output: [1, 'bar', 9, 4, 'foo', 8, 5, 6, 7]

There are a few possibilities...

from itertools import chain, izip_longest
def alternate(a, b):
    for i in range(max(len(a), len(b))):
        if i < len(a):
            yield a[i]
        if i < len(b):
            yield b[i]

def alternate2(list_a, list_b):
    unfound = {}
    for a, b in izip_longest(list_a, list_b, fill=unfound):
        if a is not unfound:
            yield a
        if b is not unfound:
            yield b

a = [1,9,'foo',5,7]
b = ['bar',4,8,6]
print list(alternate(a, b))
print list(alternate2(a, b))
print list(chain.from_iterable(zip(a, b))) + a[len(b):] + b[len(a):]


Get device orientation I feel like I can vastly improve this. Any ideas?

// get orientation of device
getOrientation();

// animate
var num = 400;

if( $('body').hasClass("landscape") ) {
    $('.example').animate({'bottom', 0});
} else {
    $('.example').animate({'bottom', num});
}

function getOrientation(){

    switch(window.orientation) {
     case -90:
     case 90:
    $('body').addClass("landscape");
        // alert('landscape');
        break;
     default:
       //alert('portrait');
       $('body').addClass("portrait");
       break;
     }
}

window.onorientationchange = function() {
  getOrientation();
};

Well for starters bind the event using the jQuery standard

var $win = $(window).bind('orientationchange', function(){
    // math.abs to reduce logical operators
    $('body').removeClass('landscape portrait') // reset
        .addClass(Math.abs(this.orientation) === 90 ? "landscape" : "portrait");
});

// now you can trigger the orientation manually on doc ready to set the class
$win.trigger('orientationchange');

var num = 400,
    $body = $('body'),
    $example = $('.example');

if($body.hasClass("landscape"))
    $example.animate({'bottom', 0});
else
    $example.animate({'bottom', num});


Breadth-first search in Java: competitive style Here my attempt was to write a graph algorithm the way a competitive programmer would write under time pressure. The problem to solve was to find a shortest path in a directed, unweighted graph using breadth-first search algorithm (BFS, for short).

Basically, I would like to hear comments on my style. Is it effective, for example?

import java.util.Arrays;

class BFS {

    static int[] bfs(int[][] graph, int sourceNode, int targetNode) {
        int[] queue = new int[graph.length];
        int[] distance = new int[graph.length];
        int[] parents = new int[graph.length];

        for (int i = 0; i < parents.length; i++) {
            parents[i] = -1;
        }

        int queueStartIndex = 0;
        int queueEndIndex = 1;

        queue[0] = sourceNode;
        distance[sourceNode] = 0;

        while (queueStartIndex < queueEndIndex) {
            int currentNode = queue[queueStartIndex++];

            if (currentNode == targetNode) {
                return buildPath(targetNode,
                                 distance[targetNode] + 1,
                                 parents);
            }

            for (int childNode : graph[currentNode]) {
                if (parents[childNode] == -1) {
                    parents[childNode] = currentNode;
                    distance[childNode] = distance[currentNode] + 1;
                    queue[queueEndIndex++] = childNode;
                }
            }
        }

        return null;
    }

    private static int[] buildPath(int targetNode,
                                   int pathLength,
                                   int[] parents) {
        int[] path = new int[pathLength];
        int pathIndex = path.length - 1;
        int currentNode = targetNode;

        while (currentNode != -1) {
            path[pathIndex--] = currentNode;
            currentNode = parents[currentNode];
        }

        return path;
    }

    /*    B ----+
         /      |
        A       E
         \      /
          C - D
    */

    public static void main(String[] args) {
        int a = 0;
        int b = 1;
        int c = 2;
        int d = 3;
        int e = 4;

        int[][] graph = new int[5][];
        graph[a] = new int[]{ c, b };
        graph[b] = new int[]{ e };
        graph[c] = new int[]{ d };
        graph[d] = new int[]{ c, e };
        graph[e] = new int[]{ b, d };

        // A -> B -> E
        int[] path = bfs(graph, a, e);
        System.out.println(Arrays.toString(path));

        // A <- B <- E does not exist:
        System.out.println(Arrays.toString(bfs(graph, e, a)));
    }
}

(See the next iteration.)

Correctness.

The parent of the source node is initially -1, and buildPath relies on that. However, if the source node belongs to a cycle, its parent will be eventually reassigned, breaking the contract. Now buildPath will misbehave.
Efficiency.

Since the algorithm assumes an unweighted graph, the distance array seems redundant. In the implementation only one value is used, only to hint buildPath on the size of the path array. Meanwhile, the distances are incremented likely \$O(V)\$ times, and surely more than P = pathLength times. Instead you can let buildPath to compute P, trading \$O(V)\$ increments for \$O(P)\$ path length computation.


web scraper for emails and links I created a class to scrape URLS, parse and validate emails and get internal links.
How can I achieve the SOLID principles in this class written in Javascript to make a web scraper?
const axios = require('axios');
const fs = require('fs')
const {
    JSDOM
} = require('jsdom');
class Scraper {
    static dangerDomains = ['loan', 'work', 'biz', 'racing', 'ooo', 'life', 'ltd', 'png'];
    static emailRegex = /([a-zA-Z0-9._-]+@[a-zA-Z0-9._-]+\.[a-zA-Z0-9._-]+)/gi;
    static domObject = null;
    static internalLinks = new Set();
    static externalLinks = new Set();
    constructor(url) {
        this.url = url;
        this.emails = new Set();
        this.dangerEmails = new Set();
    }

    async executeScraper() {
        const fetchedData = await this.fetchUrlAndGetDom(this.url);
        const links = await this.getInternalLinks();
        const linksArray = Array.from(Scraper.internalLinks);
        const emails = await this.fetchInternalLinks(linksArray);
        this.addEmails(emails);
        this.validateEmails();
        await this.writeFile();
    }

    const result = async fetchInternalLinks(internalLink) {
        const result = await Promise.all(internalLink.map((link) => {
            return this.fetchUrlAndGetDom(`${this.url}${link}`);
        }));
        return result;
    }

    async fetchUrlAndGetDom(url) {
        try {
            const response = await axios(url);
            if (response.status === 200) {
                let htmlData = await response.data;
                Scraper.domObject = await new JSDOM(htmlData);
                let dades = this.searchForEmails();
                return dades;
            } else if (response.status === 404) {
                console.log('this page doesnt exists')
                return process.exit(1);
            }
        } catch (error) {
            console.log(error)
            return process.exit(1);
        }
    }

Get all the internal links from a website to scrape them

    getInternalLinks() {
        let links = Scraper.domObject.window.document.body;
        links = links.querySelectorAll('a[href^="/"]');
        Array.from(links).filter(link => Scraper.internalLinks.add(link.getAttribute('href')));
    }

Use of a regex to find all the emails in the dom

    searchForEmails() {
        let emailsInDom = Scraper.domObject.window.document.body.innerHTML;
        let emails = emailsInDom.toString().match(Scraper.emailRegex)
        return emails;
    }

Validate all the emails found in the dom

    validateEmails() {
        const validatedEmails = Array.from(this.emails).filter(email => {
            let domainName = email.split('@')[1].split('.')[1];
            if (Scraper.dangerDomains.includes(domainName)) {
                this.dangerEmails.add(email);
                this.emails.delete(email);
            } else return email;
        });
    }

Write the emails in a file

    async writeFile() {
        return new Promise((resolve, reject) => {
            fs.writeFile('./emails.txt', Array.from(this.emails), err => {
                if (err) reject('Could not write file');
                resolve('success');
            });
        });
    }

Add email to the propertie emails

    addEmails(emailsInDom) {
        Array.from(emailsInDom).filter(el => {
            if(el !== null) {
                el.filter(el => this.emails.add(el))
      }
    });
    }
}

This execute the scraper

const Scraper = require('./Scraper');
const scraper = new Scraper('any url');
(async () => {
  try {
    const dades = await scraper.executeScraper();

  } catch (e) {
    console.log(e)
  }
})()

I choose to focus on the Single Responsibility Principle in this answer.
To move closer to the Single Responsibility Principle, I think some of the functionality needs to be moved around. I have decomposed the class and removed all state that is shared on the class (rhetorical question: is the class responsible for state or functionality? If it is "both", then surely that violates SRP?!). Shared state like this can get tough to reason about and lead to an ever-larger class as the functionality evolves over time.
How about a scraper object, with one method run that accepts a URL and returns the result for a URL (and its sub-URLs)?
Usage:
import scraper from './scraper.js'

const { emails: { valid, dangerous }, links: { internal } } = await scraper.run('http://www.example.com')

We configure a result object which we then pass around to populate, before it is returned:
const scraper = {
    async run(url) {
        const result = {
            emails: { valid: [], dangerous: [] },
            links: { internal: [] }
        }
        return scrape(url, result)
    }
}

We define a scrape function, responsible for coordinating the scraping of a URL:
async function scrape(url, result, visited=new Set()) {
    if (visited.has(url)) {
        return result
    }

    visited.add(url)
    const dom = await fetchDom(url)
    const internalLinks = findInternalLinks(dom)
    const { valid, dangerous } = validateEmails(findEmails(dom))

    for (let link of internalLinks) {
        await scrape(link, result, visited)
    }

    result.links.internal.push(...internalLinks)
    result.emails.valid.push(...valid)
    result.emails.dangerous.push(...dangerous)

    return result
}

Function fetchDOM is responsible for retrieving the DOM from the network for a URL:
async function fetchDOM(url) {
    try {
        const { status, data } = await axios(url)
        switch (status) {
            case 200:
                return new JSDOM(await data)
            case 404:
                console.log('this page doesnt exists')
                return process.exit(1)
        }
    } catch (error) {
        console.log(error)
        return process.exit(1)
    }
}

findInternalLinks is responsible for returning an array of internal links, found within the supplied DOM:
function findInternalLinks({ window: { document: body } }) {
    let links = body.querySelectorAll('a[href^="/"]')
    return Array.from(links).map(link=>link.getAttribute('href'))
}

findEmails is responsible for returning the an array of emails, found within the supplied DOM:
const EMAIL_REGEX = /([a-zA-Z0-9._-]+@[a-zA-Z0-9._-]+\.[a-zA-Z0-9._-]+)/gi

function findEmails({ window: { document: { body: { innerHTML: html } } }) {
    return html.toString().match(EMAIL_REGEX)
}

validateEmails splits an array of emails into two sets: valid and dangerous:
const DANGEROUS_DOMAINS = ['loan', 'work', 'biz', 'racing', 'ooo', 'life', 'ltd', 'png']

function validateEmails(emails) {
    const dangerous = []
    const valid = Array.from(emails).filter(email=>{
        let domainName = email.split('@')[1].split('.')[1]
        if (DANGEROUS_DOMAINS.includes(domainName)) {
            dangerous.add(email)
            return false
        } else
            return true
        })
    return { valid, dangerous }
}

The writing of a file is a wholly separate responsibility. Perhaps create an emailFileWriter object to encapsulate it:
const emailFileWriter = {
    write(emails) {
        return new Promise((resolve,reject)=>{
            fs.writeFile('./emails.txt', emails, err=>{
                if (err) {
                    reject('Could not write file')
                }
                resolve('success')
            })
        })
    }
}

I would put each of these functions and objects in a separate file. Each has its own responsibility after all.