Animations with vanilla JS I've used jQuery for a long time, and now I'm going to do some animations with vanilla JS. Here's my code so far:

var box = document.getElementById('box'),
    a1end,
    a2end,
    cdend,
    a3end,
    a4end,
    animate1 = setInterval(function() {
        if (box.style.top === (window.innerHeight - 120) + 'px') {
            clearInterval(animate1);
            a1end = true;
        }
        box.style.top = (
            +box.style.top.replace('px', '') + 1
        ) + 'px';
    }, 1),
    animate2 = setInterval(function() {
        if (box.style.left === (window.innerWidth - 120) + 'px') {
            clearInterval(animate2);
            a2end = true;
        }
        box.style.left = (
            +box.style.left.replace('px', '') + 1
        ) + 'px';
    }, 1);

checkEnd1(function() {
    box.style.background = 'greenyellow';
    box.innerText = 'Next stage starts in 5 seconds';
    var i = 4,
        cd = setInterval(function() {
            if (!i) {
                clearInterval(cd);
                cdend = true;
            }
            box.innerText = 'Next stage starts in ' + i + ' seconds';
            i--;
        }, 1000);

});
checkEnd2(function() {
    box.style.background = 'white';
    box.innerText = '';
    var animate3 = setInterval(function() {
        if (box.style.left === '20px') {
            clearInterval(animate3);
            a3end = true;
        }
        box.style.left = (
            +box.style.left.replace('px', '') - 1
        ) + 'px';
    }, 1);
});
checkEnd3(function() {
    var animate4 = setInterval(function() {
        if (box.style.top === '20px') {
            clearInterval(animate4);
            a4end = true;
        }
        box.style.top = (
            +box.style.top.replace('px', '') - 1
        ) + 'px';
    }, 1);
});
checkEnd4(function() {
    box.style.background = 'lightcyan';
    box.innerText = 'Animation complete';
});

function checkEnd1(fn) {
    (a1end && a2end) ? fn() : setTimeout(function() {
        checkEnd1(fn);
    }, 200);
}
function checkEnd2(fn) {
    (cdend) ? fn() : setTimeout(function() {
        checkEnd2(fn);
    }, 200);
}
function checkEnd3(fn) {
    (a3end) ? fn() : setTimeout(function() {
        checkEnd3(fn);
    }, 200);
}
function checkEnd4(fn) {
    (a4end) ? fn() : setTimeout(function() {
        checkEnd4(fn);
    }, 200);
}

jsFiddle demo

I've also noticed that the first animation isn't working as excepted. Out of curiousity, I tried it with jQuery's .animate() and it worked as excepted:

$('#box').animate({
    top: window.innerHeight - 120,
    left: window.innerWidth - 120
});

Note: please don't tell me to use jQuery or other libs, I want to use vanilla.

Bounty Edit

Currently, the animation goes diagonally for a while, then when it reaches the bottom, it continues to go sideways. If you tell me how to make it go diagonally all the time, I'll give you a bounty.

Rather than use your code as a base, I chose to rewrite from scratch to incorporate a couple of ideas to get it to move diagonally properly, as well as hopefully make some cleaner code.

var speed = 2;

function animateTo(dom_elem, x, y, finishedCallback) {
    var pos = {
        x: dom_elem.offsetLeft,
        y: dom_elem.offsetTop
    };

    function animate() {
        var xToTarget = x - pos.x;
        var yToTarget = y - pos.y;

        xToTarget = Math.max(0, xToTarget);
        yToTarget = Math.max(0, yToTarget);

        if (xToTarget == 0 && yToTarget == 0) {
            finishedCallback();
        }

        var scale = speed/Math.sqrt(Math.pow(xToTarget, 2) + Math.pow(yToTarget, 2));

        var delta_x = xToTarget * scale;
        var delta_y = yToTarget * scale;

        pos.x += delta_x;
        pos.y += delta_y;

        pos.x = Math.min(x, pos.x);
        pos.y = Math.min(y, pos.y);

        dom_elem.style.left = pos.x + 'px';
        dom_elem.style.top = pos.y + 'px';

        setTimeout(animate, 1);
    }

    setTimeout(animate, 1);
}

var box = document.getElementById('box');
animateTo(box, window.innerWidth - box.offsetWidth, window.innerHeight - box.offsetHeight, function() {
    alert('done');
});

Firstly, I have abstracted it into the idea of animating a dom element to a particular position. You can then chain together the animations needed using the finishedCallback.

The trick to getting it to animate diagonally properly, instead of hitting the bottom and sliding along, is to move along x and y by different amounts depending on the ratio needed to reach the target instead of always incrementing by 1.

Imagine the current position of the box as one point on the hypotenuse of a triangle, and the destination as the other end of the hypotenuse. The distance along the x-axis forms one of the other edges, and the distance on the y-axis the final edge.

The hypotenuse shows us the line we want the box to travel along. But we don't want to travel there all in one go, we want to move along it slowly. If we take the triangle we have formed, and scale it down the angle of the hypotenuse is still the same, meaning if we keep moving in that direction we will eventually reach the end point, but the distance on the x and y axis are much smaller.

These smaller x and y distances (or deltas) are what we want to add to our position instead of 1.

If we were trying to animate from A to B, the red line shows how you would move if you always added 1. The dotted lines show the scaled triangles.

Math.sqrt(Math.pow(xToTarget, 2) + Math.pow(yToTarget, 2)) calculates the length of the hypotenuse in our triangle from current position to the target. We then use speed/length to create the scale we multiply the x and y lengths by. In this way the box will always move the equivalent of speed pixels diagonally towards the target.

I chose to keep track of the x and y position manually instead of reading it back from the dom element because I think it makes the code cleaner, and I think it should be faster.


Converting an image to a bitmap What the code does is convert an image to a bitmap without having to create 3 arrays to make the conversion. It basically uses 1/3rd of the memory that it would normally use.

I'm trying to refactor this method to make it shorter, less complex and more readable. So far, I've been unsuccessful in the 4 attempts I've made. Every time I try to extract something it says that there are ambiguous return statements.

private Bitmap decodeBitmapFromFile(File file, boolean preview) {
  byte[] byteArr = new byte[0];
  byte[] buffer = new byte[1024];
  int count = 0;
  int len;
  InputStream inputStream = null;

  try {
    inputStream = new FileInputStream(file);
    while ((len = inputStream.read(buffer)) > -1) {
      if (len != 0) {
        if (count + len > byteArr.length) {
          byte[] newBuf = new byte[(count + len) * 2];
          System.arraycopy(byteArr, 0, newBuf, 0, count);
          byteArr = newBuf;
        }

        System.arraycopy(buffer, 0, byteArr, count, len);
        count += len;
      }
    }

    final BitmapFactory.Options options = new BitmapFactory.Options();
    options.inJustDecodeBounds = true;
    BitmapFactory.decodeByteArray(byteArr, 0, count, options);

    options.inPurgeable = true;
    options.inInputShareable = true;
    options.inJustDecodeBounds = false;
    if (preview) {
      options.inSampleSize = getInSampleSize();
    }
    options.inPreferredConfig = Bitmap.Config.ARGB_8888;

    return BitmapFactory.decodeByteArray(byteArr, 0, count, options);
  } catch (FileNotFoundException e) {
    e.printStackTrace();
  } catch (IOException e) {
    e.printStackTrace();
  } finally {
    try {
      inputStream.close();
    } catch (Exception e) {
    }
  }

  return null;
}

A couple of thoughts.

Always think about breaking your code into discrete units of work, this will make it more logical, more readable and importantly, more testable. In your case, separate out the reading of the data from the creation of the bitmap.
The ambiguous return will either be because you are modifying multiple local properties (count, len, buffer) or because you return a new Bitmap or null from different points in your code. By needing to use count in your Bitmap creation you make it harder to refactor.
If you are concerned about 0 length returns from inputStream.read() consider adding a handbrake to avoid infinite loops - I have never encountered this in real life  though.
Does your code perform better than using ByteArrayOutputStream, something like the below.

Code:

//add throws or try/catch as suits your code.

private byte[] readBitmapData() {
    ByteArrayOutputStream buffer = new ByteArrayOutputStream();
    byte[] inputBuffer = new byte[16384]; //16K default buffer size
    int nRead;

    while ((nRead = is.read(inputBuffer, 0, data.length)) != -1) {
        buffer.write(inputBuffer, 0, nRead);
    }

    return buffer.toByteArray();
}

You can check out the OpenJDK code for ByteArrayOutputStream on docjar. Note their code for grow and ensureCapacity.


Game of Life Neighbour count optimization This is a working Neighbours method. Is there any way to make this simpler and concise?

public int Neighbours(int xCoordinate, int yCoordinate)
        {
            xCoordinate = xCoordinate -1;
            yCoordinate = yCoordinate -1;
            int NeighbourCounter =0;
            int incrementer = 0;
            int i1 =-1;
            int j1 =-1;
            int i2 =1;
            int j2 =1;

            if(yCoordinate==0)
            {
                j1=1;
            }

            if(xCoordinate==0)
            {
                i1=1;
            }

            if(yCoordinate+1==xLen)
            {
                j2=0;
            }

            if(xCoordinate+1==yLen)
            {
                i2=0;
            }

            for(int j=j1; j<=j2; j++)
            {
                if(j==0)
                {
                    incrementer = 2;
                }
                else
                {
                    incrementer = 1;
                }

                for(int i=i1; i<=i2; i+=incrementer)
                {
                    if(CG[yCoordinate + j][xCoordinate + i] == 1)
                    {
                        NeighbourCounter++;
                    }
                }
            }
                return NeighbourCounter;
        }

There are several style issues with this code. Most notably:

Use camelCase for method names
Use camelCase for variable names
Put spaces around operators, for example:

i = i1 instead of i=i1
yCoordinate + 1 == xLen instead of yCoordinate+1==xLen
... and so on

j1, j2, ... are poor variable names

To simplify this code, a couple of ideas:

Use an enum to represent neighbors: North, NorthEast, East, SouthEast, ...

Create custom fields dx and dy to represent the direction offset

Iterate over Neighbor.values(), for each item perform this check:

Calculate the neighbor position using current position + the neighbor's dx and dy
Check if the position is on the grid, for example with a isValid(Position pos, Neighbor neighbor) method
Check if the position is alive, for example with a isAlive(Position pos, Neighbor neighbor) method
If both the above are true, increment the count

To get you started, here's an example enum:

enum Neighbor {
    North(0, 1),
    NorthEast(1, 1),
    East(1, 0);

    private final int dx;
    private final int dy;

    Neighbor(int dx, int dy) {
        this.dx = dx;
        this.dy = dy;
    }
}


Unity and an IoC for Settings After a somewhat lengthily discussion in the chat room, I have set up an IoC container with Unity to store my settings in.

This is my static IoC class:

public static class IoCContainer
{
    public static IUnityContainer Container = new UnityContainer();
}

In App.xaml.cs:

IoCContainer.Container.RegisterType<ApplicationSettingProvider<Theme>, ApplicationThemeProvider>(new ContainerControlledLifetimeManager());
IoCContainer.Container.RegisterType<ApplicationSettingProvider<FontSize>, ApplicationFontSizeProvider>(new ContainerControlledLifetimeManager());
IoCContainer.Container.RegisterType<ApplicationSettingProvider<AppVersion>, ApplicationStartupVersionProvider>(new ContainerControlledLifetimeManager());

Wherever I want to access my settings:

private ApplicationSettingProvider<Theme> _ThemeProvider = IoCContainer.Container.Resolve<ApplicationSettingProvider<Theme>>();
private ApplicationSettingProvider<FontSize> _FontSizeProvider = IoCContainer.Container.Resolve<ApplicationSettingProvider<FontSize>>();
private ApplicationSettingProvider<AppVersion> _StartupVersionProvider = IoCContainer.Container.Resolve<ApplicationSettingProvider<AppVersion>>();

And, an example setting, just to make everything complete:

public class ApplicationThemeProvider : ApplicationSettingProvider<Theme>
{
    public ApplicationThemeProvider()
    {
        ApplicationData.Current.DataChanged += (a, o) =>
        {
            CoreApplication.MainView.CoreWindow.Dispatcher.RunAsync(CoreDispatcherPriority.Normal, () =>
            {
                OnSettingChanged((Theme)ApplicationData.Current.RoamingSettings.Values["Theme"]);
            });
        };
    }

    public override Theme GetCurrentSetting()
    {
        object value;
        if (ApplicationData.Current.RoamingSettings.Values.TryGetValue("Theme", out value))
        {
            int themeValue = (int)value;
            return (themeValue >= 0 && themeValue <= 2) ? (Theme)themeValue : Theme.Default;
        }
        return Theme.Default;
    }

    public override void SetCurrentSetting(Theme theme)
    {
        ApplicationData.Current.RoamingSettings.Values["Theme"] = (int)theme;
        ApplicationData.Current.SignalDataChanged();
    }
}

Which is based on:

public class SettingChangedEventArgs<T> : EventArgs
{
    public T NewSetting { get; private set; }

    public SettingChangedEventArgs(T newSetting)
    {
        NewSetting = newSetting;
    }
}

public abstract class ApplicationSettingProvider<T>
{
    public abstract T GetCurrentSetting();
    public abstract void SetCurrentSetting(T setting);

    public event EventHandler<SettingChangedEventArgs<T>> SettingChanged;
    public void OnSettingChanged(T newSetting)
    {
        var handler = SettingChanged;
        if (handler != null)
        {
            handler(this, new SettingChangedEventArgs<T>(newSetting));
        }
    }
}

This review is primarily to make sure I am using Unity right.  Please tell me everything that can be improved.

public static class IoCContainer
{
    public static IUnityContainer Container = new UnityContainer();
}

I've never used Unity, but this looks quite dangerous to me: your IoC container is accessible from anywhere in the code. Dependency Injection does not play well with anything public static - the IoC container is no exception here. In fact, the IoC container instance belongs in one, single place: your composition root - and that's app.xaml.xs. So instead of this:

IoCContainer.Container.RegisterType<ApplicationSettingProvider<Theme>, ApplicationThemeProvider>(new ContainerControlledLifetimeManager());
IoCContainer.Container.RegisterType<ApplicationSettingProvider<FontSize>, ApplicationFontSizeProvider>(new ContainerControlledLifetimeManager());
IoCContainer.Container.RegisterType<ApplicationSettingProvider<AppVersion>, ApplicationStartupVersionProvider>(new ContainerControlledLifetimeManager());

You should have that:

_container.RegisterType<ApplicationSettingProvider<Theme>, ApplicationThemeProvider>(new ContainerControlledLifetimeManager());
_container.RegisterType<ApplicationSettingProvider<FontSize>, ApplicationFontSizeProvider>(new ContainerControlledLifetimeManager());
_container.RegisterType<ApplicationSettingProvider<AppVersion>, ApplicationStartupVersionProvider>(new ContainerControlledLifetimeManager());

Where _container is an instance field of your App class. In a wpf application this setup code would be in the OnStartup override: the UnityContainer instance only ever lives at the composition root level.

Speaking of composition, you've either not included it, or all you're doing is the registration part. And this explains it:

  Wherever I want to access my settings:

private ApplicationSettingProvider<Theme> _ThemeProvider = IoCContainer.Container.Resolve<ApplicationSettingProvider<Theme>>();
private ApplicationSettingProvider<FontSize> _FontSizeProvider = IoCContainer.Container.Resolve<ApplicationSettingProvider<FontSize>>();
private ApplicationSettingProvider<AppVersion> _StartupVersionProvider = IoCContainer.Container.Resolve<ApplicationSettingProvider<AppVersion>>();

That's exactly the opposite of dependency-injection: everywhere you're accessing your settings, you have a tight coupling with that static class IoCContainer and its public static IUnityContainer field.

That's the Service Locator anti-pattern: anyone, anywhere, has access to anything that's registered with the IoC container. The typical Service Locator passes the container around as a dependency - but your is worse, because being a public static utility class, it becomes an ambient context that's accessible at any given time by anyone that wants to use it.

IoC containers are much, much more powerful than that. IoC stands for Inversion of Control - that means your types don't ask the container to resolve an instance of a service, but rather they tell it what their dependencies are, and the IoC container injects them... usually through the constructor.

The Hollywood Principle: Don't Call Them, They'll Call You

I'm going to have to make some things up here, because you've supplied us with the dependencies, but not with anything that depends on them.

So let's say your application is controller by a MainWindowViewModel.

This MainWindowViewModel class needs to access the settings; its constructor will statically document this fact, by taking in an interface that represents some SettingsProvider - here you've done that with an abstract class:

private readonly ApplicationSettingProvider<Theme> _themeSettings;
private readonly ApplicationSettingProvider<FontSize> _fontSizeSettings;
private readonly ApplicationSettingProvider<AppVersion> _appVersionSettings;

private readonly IView _view;

public MainWindowViewModel(ApplicationSettingProvider<Theme> themeSettings,
                           ApplicationSettingProvider<FontSize> fontSizeSettings,
                           ApplicationSettingProvider<AppVersion> appVersionSettings,
                           IView view)
{
    _themeSettings = themeSettings;
    _fontSizeSettings = fontSizeSettings;
    _appVersionSettings = appVersionSettings;

    _view = view;
}

The class may have more dependencies - the key is that it receives them all into its constructor.

  Constructor Bloating

  Everyone has their threshold. For most people, 3-4 constructor parameters is the upper limit, 5 in the extreme cases. When you start seeing that your constructor has too many dependencies, it's a sign that your class is responsible for too many things - you should then extract a class, wrap it with an interface, and inject that interface instead. In this case there could be a single IAppSettings abstraction injected there, and the implementation for that interface would receive the 3 SettingsProvider<T> into its own constructor.

  But let's say these are the only 3 dependencies you have to deal with for now... plus the IView.

When you register the types with your IoC container, you're telling it what types to use to resolve the dependencies when you compose the application - that's the step you missed.

Back to the Composition Root

_container.RegisterType<ApplicationSettingProvider<Theme>, ApplicationThemeProvider>(new ContainerControlledLifetimeManager());
_container.RegisterType<ApplicationSettingProvider<FontSize>, ApplicationFontSizeProvider>(new ContainerControlledLifetimeManager());
_container.RegisterType<ApplicationSettingProvider<AppVersion>, ApplicationStartupVersionProvider>(new ContainerControlledLifetimeManager());

// again, I don't know how Unity works - this is just for the concept:
_container.RegisterType<MainWindowViewModel,MainWindowViewModel>(new ContainerControlledLifetimeManager());
_container.RegisterType<IView,MainWindow>(new ContainerControlledLifetimeManager());

Now the IoC container knows about MainWindowViewModel, and all of its dependencies. The last thing to do is to resolve them, and get your app started:

var viewModel = _container.Resolve<MainWindowViewModel>();
viewModel.Show();

What just happened here? The container used the configurations you gave it, and constructor-injected all the dependencies of the viewModel - that's why that method is called Resolve: it resolves the entire application's depedency graph with a single call.

The only place you're allowed to do this is in the composition root!

That's why I said in chat, that you cannot do Dependency Injection halfway: you need to go DI all the way, and register all dependencies of every single type; every type needs to have its dependencies constructor-injected, otherwise you're breaking the pattern and you're going to have to new things up manually and thus introduce tight coupling.


Project Euler Problem #277 I am trying to solve Euler project problem 277 using python 3.8. First, I will show my program and than ask specific questions for enhancements.
The problem is a modified version of Collatz sequence. The general rule of the new one is:

\$ a_{n+1} = \, \,\, \frac {a_n} 3 \quad\$  if \$ a_n \$  is
divisible by 3. We shall denote this as a large downward step, "D".

\$ a_{n+1} = \frac {4 a_n+2} 3 \, \, \$ if \$ a_n \$   divided by 3
gives  a remainder of \$ 1 \$ . We shall denote this as an upward
step, "U".

\$ a_{n+1} = \frac {2 a_n -1} 3 \, \, \$ if \$ a_n \$   divided by 3
gives a remainder of \$ 2 \$ . We shall denote this as a small
downward step, "d".

For instance if \$ a_1=231\$ , then the sequence \$ \{a_n\}=\{231,77,51,17,11,7,10,14,9,3,1\} \$ corresponds to the steps "DdDddUUdDD".
The problem asks What is the smallest \$ a_1 > 10^{15}\$ that begins with the sequence "UDDDUdddDDUDDddDdDddDDUDDdUUDd"?
My code:
def modifCollatz(x, seq, num):
   if num == 0:
       return seq

   if x % 3 == 0:
       num -= 1
       x = x / 3
       seq.append("D")
       return modifCollatz(x, seq, num)

   elif x % 3 == 1:
       num -= 1
       x = (4 * x + 2) / 3
       seq.append("U")
       return modifCollatz(x, seq, num)

   else:
       x = (2 * x - 1) / 3
       num -= 1
       seq.append("d")
       return modifCollatz(x, seq, num)

wordList = []
for i in "UDDDUdddDDUDDddDdDddDDUDDdUUDd":
   wordList.append(i)

n = 10 ** 15
while True:
   if modifCollatz(n, [], 30)[0:30] == wordList:
       print(n)
       break
   else:
       n += 3

That works. I have tested using the example given in problem 277.

I am aware that it's using brute force and don't know what can be done to improve efficiency. I was just able to think of adding solely finding the first 30 elements of each number and incrementing by 3 n += 3, which are probably the easiest ones. What can be done additionally?

Creating a list out of a string
wordList = []
for i in "UDDDUdddDDUDDddDdDddDDUDDdUUDd":
   wordList.append(i)

can be shortened to:
wordList = list("UDDDUdddDDUDDddDdDddDDUDDdUUDd")

Algorithm
The first idea is to check the sequence letter by letter. This is more efficient than generating the sequence for a number x and then see if it matches the target sequence.
To find the number that matches Dd starting from 3, it would be:

x = 3
3 % 3 == 0, so it matches D
x = 3 / 3 = 1
1 doesn't match d, so try next number
x = 3 + 1 = 4
4 doesn't match D
x = 4 + 1 = 5
5 doesn't match D
x = 5 + 1 = 6
6 matches D
x = 6 / 3 = 2
2 matches d
Solution is 6

Second idea is to filter out numbers that cannot be part of the solution.
In the previous example, numbers 4 and 5 can be filtered out. Similarly to your n += 3. Let's see some examples:
Numbers that match the prefix D:
3,6,9,12,15,18,21...

Numbers that match the prefix DD:
9,18,27,36...

Numbers that match the prefix DDD:
27,54,81,...

Each sequence is a subset of the previous.
Numbers in the sequence differ by \$3^i\$, with i starting from 1.

Numbers that match the prefix U:
4,7,10,13,16,19,21,...

Numbers that match the prefix UU:
7,16,25,34,...

Numbers that match the prefix UUU:
25,52,79,...

Each sequence is a subset of the previous.
Numbers in the sequence differ by \$3^i\$, with i starting from 1.

Same for sequences that match d. Let's see a mixed prefix.
Numbers that match the prefix D:
3,6,9,12,15,18,21...

Numbers that match the prefix DU:
12,21,30,...

Each sequence is a subset of the previous.
Numbers in the sequence differ by \$3^i\$, with i starting from 1.


Resize the hash table and rehash every element I'm trying to increase my hash table size by two if the table is filled more than half of it. I tried to rehash every single element from the old table and put it back to my new table, then assign my old table as the new table again. What do you guys think of my own created function? I'm practicing for my final exam and only creating this in paper. I haven't tried yet using compiler and so forth.

I'm using quadratic probing to insert the keys, and once more than half of the table is filled, resize the hash table by calling the function resize().

//find index
int HashingTable::hash(int x, int size){
     return x % size;
}

//insert value to the array, but using quadratic probing
void HashingTable::insertQuadratic(int x, int T[]) {
    int index = hash(x, size);
    int i = 1;
    //the quadratic probing
    while (T[index] != empty) {
        index = (index + (i*i)) % size; //increment quadraticly
        i = i + 1;
    }
    T[index] = x;
    numOfItems = numOfItems + 1;
    double loadFactor = numOfItems / size;
    if (loadFactor >= 0.5) {
        resize(T);
    }
}

void HashingTable::resize(int T[]) {
    int newSize = 2 * size;
    int* newTable = new int[newSize];
    for (int i = 0; i < newSize; i++) {
        newTable[i] = empty;
    }
    for (int i = 0; i < newSize; i++) {
        if (T[i] != empty) {
            int index = hash(T[i], newSize);
            int i = 1;
            while (newTable[index] != empty) {
                index = (index + (i*i)) % newSize;
            }
            newTable[index] = T[i];
        }
    }
    T = newTable;
    delete newTable;
}

Advice 1

for (int i = 0; i < newSize; i++) {
    if (T[i] != empty) {

Did you intend to iterate only over size entries from T? If so, you need:

for (int i = 0; i < size; i++) {
    if (T[i] != empty) {

Advice 2

T = newTable;
delete newTable;

You deallocate the actual new table array. Also, since you don't delete the table pointed to by T, you leak memory. I think your intention was to write:

delete[] T;
T = newTable;

Advice 3

T is not a good name for a hash table, since people can confuse it with a type parameter. Consider using something like storage_array, storage_table, table, or similar.


Recyclerview Adapter for weather app - Makes API calls for each item I have a weather app that allows the user to save Cities (with nickname and zip) into a room database.  I'm using mvvm and livedata to observe the list of cities and update the recyclerview accordingly. I then perform an API call for each item in the recyclerview, which to me seems wrong.  It works but seems wrong.

I'd appreciate any reviews on things I could do to improve.

public class RecyclerAdapter extends ListAdapter<MyCity, RecyclerAdapter.ViewHolder> {

    private static final DiffUtil.ItemCallback<MyCity> DIFF_CALLBACK = new DiffUtil.ItemCallback<MyCity>() {
        @Override
        public boolean areItemsTheSame(@NonNull MyCity oldItem, @NonNull MyCity newItem) {
            return oldItem.getId() == newItem.getId();
        }

        @Override
        public boolean areContentsTheSame(@NonNull MyCity oldItem, @NonNull MyCity newItem) {
            return oldItem.getZipCode() == newItem.getZipCode();
        }
    };

    public RecyclerAdapter() {
        super(DIFF_CALLBACK);
    }

    @NonNull
    @Override
    public ViewHolder onCreateViewHolder(@NonNull ViewGroup parent, int viewType) {
        View view = LayoutInflater.from(parent.getContext()).inflate(R.layout.layout_item_list, parent, false);
        return new ViewHolder(view);
    }

    @Override
    public void onBindViewHolder(@NonNull ViewHolder holder, int position) {

        MyCity city = getItem(position);

        WeatherApi weatherApi = RetrofitService.createService(WeatherApi.class);

        weatherApi.getWeatherWithZip(city.getZipCode(), Constants.API_KEY).enqueue(new Callback<WeatherResponse>() {
            @Override
            public void onResponse(Call<WeatherResponse> call, Response<WeatherResponse> response) {
                if (response.isSuccessful()) {
                    String zip = String.valueOf(city.getZipCode());
                    holder.zip.setText(zip);
                    holder.city.setText(response.body().getName());
                    holder.weather.setText(Conversions.kelvinToFahrenheit(response.body().getMain().getTemp()) + "°C");
                }
            }

            @Override
            public void onFailure(Call<WeatherResponse> call, Throwable t) {

            }
        });
    }

    public MyCity getCityAt(int position) {
        return getItem(position);
    }

    class ViewHolder extends RecyclerView.ViewHolder {

        private TextView zip, city, weather;

        public ViewHolder(@NonNull View itemView) {
            super(itemView);
            zip = itemView.findViewById(R.id.tv_zip);
            city = itemView.findViewById(R.id.tv_city);
            weather = itemView.findViewById(R.id.tv_weather);
        }
    }
}

EDIT:
I just got an email from openweatherapi... I made 11791 calls in a minute. This is definitely not ideal.

First issue

You're calling the weather API on every OnBindViewHolder(), which means it will get called for each visible row, and on new rows entering, or re-entering the screen. So if you scroll the list a few times you will call the API for each city more than once.

Second issue

Your Adapter shouldn't make any API calls. In fact, it should only concern itself about displaying cities.

How can we solve this?

Your MyCity model should provide all information necessary to display the weather.

So let's add city name and temperature to your model.

Next we create a service which iterates over the cities and calls the API once for each city. This can be a regular Android Service for example, or something more advanced like WorkManager.

This logic will be triggered from your ViewModel.

Depending on how frequent you want the weather updates to be, you can call this each time you open the screen, or on certain intervals.

In all cases, once you get the response back, you need to persist the new data to your database.

Since you are observing the data, the adapter will be automatically notified of changes.

Note: when retrieving the cities to call the API you shouldn't use LiveData because when you update the city it will re-trigger the LiveData and you find yourself in an endless loop.

Your Dao will look something like this:

@Dao
public interface CitiesDao {

    @Query("SELECT * FROM cities")
    List<MyCity> getCities();

    @Query("SELECT * FROM cities")
    LiveData<List<MyCity>> observeCities();

    // etc

}

This is just the starting point.

More things you can consider:

Use different models for data and UI (e.g. MyCity and MyCityUiModel)
Use the Repository pattern


Project Euler 22: Names scores using dictionary I solved Project Euler 22 using Python 3. The problem reads as follows:

  Using names.txt (right click and 'Save Link/Target As...'), a 46K text
  file containing over five-thousand first names, begin by sorting it
  into alphabetical order. Then working out the alphabetical value for
  each name, multiply this value by its alphabetical position in the
  list to obtain a name score.

  For example, when the list is sorted into alphabetical order, COLIN,
  which is worth 3 + 15 + 12 + 9 + 14 = 53, is the 938th name in the
  list. So, COLIN would obtain a score of 938 × 53 = 49714.

  What is the total of all the name scores in the file?

As you can see, the problem has much to do with handling lists of items and not so much with mathematics, unlike most other Euler problems. I chose to break the list of names, with the index/rank of each name as the key.

Are there ways to improve its performance that you can see? I'm also interested in any other possible improvements in style, naming, PEP8, etc.

'''
Project Euler 22: Names scores
https://projecteuler.net/problem=22
'''
import string
from typing import List, Dict

def get_names_from_file(file_path: str) -> List:
    '''
    Parses a text file containing names in this format:
    "FOO","BAR","BAZ"
    and returns a list of strings, e.g.,
    ["FOO","BAR","BAZ"]
    '''
    with open(file_path, "r") as file_:
        return file_.read().replace("\"", "").split(",")

def get_sorted_indexed_dict(items: List, is_0_indexed: bool = False) -> Dict:
    '''
    Takes a list of strings and returns an alphabetically sorted dict
    with the item's 1-indexed (by default) position as key, e.g.,
    `list` arg: [ "FOO", "BAR", "BAZ" ]
    returns: { 1 : "BAR", 2 : "BAZ", 3 : "FOO" }
    The return dict can be 0-indexed by adding a 2nd argument as True.
    '''
    ix_start = 1
    if is_0_indexed:
        ix_start = 0
    items = sorted(items)
    numbered_dict = {}
    for item in items:
        numbered_dict[items.index(item) + ix_start] = item
    return numbered_dict

def get_alpha_values() -> Dict:
    '''
    Assigns ASCII chars A-Z, inclusive, to a
    number value 1-26, respectively.
    '''
    alpha_values = {}
    index = 1
    letters = list(string.ascii_uppercase)
    for letter in letters:
        alpha_values[letter] = index
        index += 1
    return alpha_values

def get_word_alpha_value(word: str, alpha_values: Dict) -> int:
    '''
    Calculates the value of each letter in the word
    and returns the sum of the letter values.
    '''
    word_value = 0
    for letter in list(word):
        word_value += alpha_values[letter]
    return word_value

def get_word_ranked_value(rank: int, word: str, alpha_values: Dict) -> int:
    '''
    Calculates the ranked value according to problem 22, i.e.,
    the value of each word's letters multiplied by its
    alphabetical rank.
    '''
    return rank * get_word_alpha_value(word, alpha_values)

def get_pe22_solution(file_path: str) -> int:
    '''
    Returns the solution to Project Euler 22 based on the file path provided.
    '''
    alpha_values = get_alpha_values()
    # sanity check some alpha values
    assert alpha_values['A'] == 1
    assert alpha_values['Z'] == 26
    assert alpha_values['S'] == 19

    names = get_names_from_file(file_path)
    names = get_sorted_indexed_dict(names)
    # sanity checks name order based on PE 22 problem description
    assert names[1] == "AARON"
    assert names[938] == "COLIN"
    assert names[5163] == "ZULMA"

    name_values = [
        get_word_ranked_value(k, v, alpha_values)
        for (k, v) in names.items()
    ]
    return sum(name_values)

if __name__ == "__main__":
    FILE = "names.txt"
    print("The answer to PE 22 is: {}".format(get_pe22_solution(FILE)))

Nice PEP8 compliant code, Phrancis. Good job! I only have two comments to make regarding that: you usually have to use triple-double quoted strings when it comes to docstrings. From the docs (PEP257):

For consistency, always use """triple double quotes""" around
docstrings. Use r"""raw triple double quotes""" if you use any
backslashes in your docstrings. For Unicode docstrings, use
u"""Unicode triple-quoted strings""" .

More, you missed one newline between your imports and the first function. It's not such a big deal but I thought it's worth mentioning it.
In this line:
...
return file_.read().replace("\"", "").split(",")

You can use single quotes to get rid of the escape character \:
return file_.read().replace('"', '').split(',')

You can also omit the read file mode as that's the default mode when you read a file:
with open(file_path) as file_:
    ...

There's more than one place where you wrote:
something = list(string)

Instead of the situation where you sorted the list, you don't need list(string) because you can already iterate a string just fine.
Use enumerate() more often.
The enumerate() function adds a counter to an iterable. That said, you can rewrite your get_alpha_values() function like this:
def get_alpha_values() -> Dict:
    alpha_values = {}
    for index, letter in enumerate(string.ascii_uppercase, 1):
        alpha_values[letter] = index
    return alpha_values

Enumerate takes a second argument which lets you start from whatever position you want (in our case, that would be 1).
We can also make use of the get() method of the dicts and make your get_word_alpha_value() more straightforward:
def get_word_alpha_value(word: str, alpha_values: Dict) -> int:
    return sum([alpha_values.get(letter, 0) for letter in word])

The get method of a dict (like for example characters) works just like indexing the dict, except that, if the key is missing, instead of raising a KeyError it returns the default value (if you call .get with just one argument, the key, the default value is None)


Grep for pattern recursive and disable file On a shared host, I'd like to setup a cron which scans folders recursively for some base64 malware strings. Therefore, I've written the following script:

#!/bin/bash

if [ $# -ne 1 ]; then
    echo $0: usage: ./findone folder_to_start_with
    exit 1
fi

folder=$1

IFS=$'\n'
searchfiles=($(grep -r -F -n -f malware-strings.dat $folder))

for (( i=0; i<${#searchfiles[@]}; i++ ));
        do
                STR=$(echo ${searchfiles[i]} | awk -F':' '{print $1}')
                    if [ -z "$STR" ];
                        then true;
                        else chmod 000 $STR;
                    fi
        done

## Do something else like mail results etc.
printf '%s\n' "${searchfiles[@]}"

My locals test are doing what I expect. If a string pattern from "malware-string.dat" is found the file permission is changed to 000. Before scanning the production sites, I wanted to ask for a code review as I'm new to Bash and do not want to mess things up. Also, your judgement of disabling the file with chmod is enough would help, or if it is advisable to move the file outside of the www directory.

Bash idiom for looping over searchfiles is

for file in $searchfiles; do
    process "$file"

Invoking awk just to print a field looks like an overkill. Consider

read -d : str rest <<< "$file"


Best way to relate methods with properties I have a class with some properties that I need to reflect over. One of the things I need, is a instance of a complex type returned for each of my properties, which means I can't just instantiate it directly in the attribute.

Now, I have so far two ways to do this. Either I can write method-names, in strings, in each attribute pointing to the method to call to get the instance, or I can use Duck Typing and add a "magic" method, that will contain information about the instance needed for each attribute.

Okay, let me show some code and then I need some feedback on which way is preferable or suggestions for other ways I haven't thought of.

public class SomeClass
{
   [Parameter("OnePropertyWidgetProvider")]
   public string OneProperty { get; set; }

   [Parameter("AnotherPropertyWidgetProvider")]
   public string AnotherProperty { get; set; }

   private Widget OnePropertyWidgetProvider() {}
   private Widget AnotherPropertyWidgetProvider() {}
}

Or

public class SomeClass
{
   public string OneProperty { get; set; }
   public string AnotherProperty { get; set; }

   private ParameterWidgets GetWidgets()
   {
      return new ParameterWidgets()
        {
            { () => this.OneProperty, new Widget() },
            { () => this.AnotherProperty, new Widget() },
        };
   }
}

The class itself a unit-of-work that can be configured through a GUI. The Widget-instance is needed to configure the editor the user should see to fill out the given property.

My concern with the first way is lack of type-safety, there is no way to check at compile-tile if the functions actually exists. It's always kinda verbose, and with many properties in one class, you end up with many methods returning Widgets.

The other way might introduce ways of coding that many programmers are not familiar with; lambdas and such. The syntax is cool, there is type safety, etc., but I'm afraid of it being "too clever".

I've done this several times with a typeof operator in the attribute. That's how MS does their Editor attribute. The only disadvantage is that you can't pass constructor attributes that way. In other words you have an attribute on your property like this:

[Widget(typeof(TextBoxWidget))]

If you need parameters, you could still do it with inheritors of Widget. Suppose you need a Min/Max/Inc on your numeric widget:

[NumericWidget(-42, 42, 1)]

The methods to get the attributes allow you to look for inheritors of an attribute type.

Without this parameterized attribute approach, I definitely prefer your second option. And I would put GetWidgets into its own IHasWidgets interface. That way you can fall back to some default all-text-box implementation when that interface is missing.

Another good option is to put this information into the property registration. Are you familiar with the DependencyProperty mechanism in WPF? The CSLA framework (among others) support a property registration mechanism that is a good match for what you're doing here.


Find max number of repeating heads and tails Problem Statement:

Consider the following example: a coin was tossed n=7 times and the results were

"Heads, Tails, Tails, Tails, Heads, Heads, Tails"

Therefore, the longest Heads streak was 2 and the longest Tails streak was 3.

Here is my code:

def getMaxStreaks(toss): #toss=> ["Heads, Tails, Tails, Tails, Heads, Heads, Tails"]
    # Return an array of two integers containing the maximum streak of heads and tails respectively
    scH, scT = 0, 0
    H, T = 0, 0
    for tossResult in range(0, len(toss)):
        if toss[tossResult] == 'Heads':
            H, T = H+1, 0
            if H > scH:
                scH = H

        elif toss[tossResult] == 'Tails':
            H, T = 0, T+1
            if T > scT:
                scT = T
    return [scH, scT] # 2 3

What can I do to improve my code, are there any improvements required? Please let me know if I need to update the question.

Don't iterate over the indices of a list only to then access the element at that index. Instead iterate directly over the elements of the list! Have a look at Loop Like A Native by Ned Halder.

In the standard library module itertools there is the groupby function, which groups equal elements together. You can use this to make your code easier:

from itertools import groupby

def max_streaks(tosses):
    longest_streaks = {"Heads": 0, "Tails": 0}
    for toss, streak in groupby(tosses):
        longest_streaks[toss] = max(longest_streaks[toss], len(list(streak)))
    return longest_streaks["Heads"], longest_streaks["Tails"]

max_streaks("Heads, Tails, Tails, Tails, Heads, Heads, Tails".split(", "))
# 2, 3

Note that this does not assume anything about tosses, unlike your code. It could be a list or a string, but also something like a generator, which has no len.

You could generalize this function to being able to count the longest streak of any elements by using a collections.defaultdict and returning it:

from collections import defaultdict
from itertools import groupby
from typing import Dict, Iterable, Hashable

def longest_streaks(elements: Iterable[Hashable]) -> Dict[Hashable, int]:
    """Count the length of the longest streak of each distinct element
    present in `elements`.
    All elements need to be hashable.
    """
    longest_streak = defaultdict(int)
    for element, streak in groupby(elements):
        longest_streak[element] = max(longest_streak[element], len(list(streak)))
    return dict(longest_streak)

if __name__ == "__main__":
    longest_streaks([1, 2, 2, 3, 3, 3, 1, 1, 1, 2, 2, 3])
    # {1: 3, 2: 2, 3: 3}

Note that I followed Python's official style-guide, PEP8, by using lower_case for variables and functions and wrapped the calling code in a if __name__ == "__main__": guard to allow importing from this script without running the example. I also added some type hints and a docstring, which makes it easier to figure out what a function does, both for anybody else reading your code and for yourself in two months.


Add an HTML body class depending on specific conditions if a block region is present on the web page or not I wrote this code and I am wondering if there is a better way to phrase this. I was considering switch case but I was not sure how to translate this code into that.

The purpose of this code is to add a css body class to the webpage depending on if certain block regions show up on the page or not. $varssignifies a path to an array available within the function. So for example preface_first is a block region within the page and I am testing to see if it is present or not. If it is, then we add an HTML body class i.e. $vars['attributes']['class'][] = for example.

/**
 * Override or insert variables into the page template.
 */
function template_preprocess_html(&$vars) {

     // Preface columns.
      if (!empty($vars['page']['preface_first']) && !empty($vars['page']['preface_second']) && !empty($vars['page']['preface_third'])) {
        $vars['attributes']['class'][] = 'preface-three';
      }
      elseif (!empty($vars['page']['preface_first']) && !empty($vars['page']['preface_second'])) {
        $vars['attributes']['class'][] = 'preface-two';
      }
      elseif (!empty($vars['page']['preface_first']) && !empty($vars['page']['preface_third'])) {
        $vars['attributes']['class'][] = 'preface-two';
      }
      elseif (!empty($vars['page']['preface_second']) && !empty($vars['page']['preface_third'])) {
        $vars['attributes']['class'][] = 'preface-two';
      }
      else {
        $vars['attributes']['class'][] = 'preface-one';
      }
}

The code works as is but I somehow think it can be improved to be shorter and more efficient.

You really do need to explain the overall purpose of your code, where this $var comes from and where this is going.

You might be suprised at the insights and thoughts you get in response.

But if we limit it to just the following code, here are my thoughts:

// Preface columns.
  if (!empty($vars['page']['preface_first']) && !empty($vars['page']['preface_second']) && !empty($vars['page']['preface_third'])) {
    $vars['attributes']['class'][] = 'preface-three';
  }
  elseif (!empty($vars['page']['preface_first']) && !empty($vars['page']['preface_second'])) {
    $vars['attributes']['class'][] = 'preface-two';
  }
  elseif (!empty($vars['page']['preface_first']) && !empty($vars['page']['preface_third'])) {
    $vars['attributes']['class'][] = 'preface-two';
  }
  elseif (!empty($vars['page']['preface_second']) && !empty($vars['page']['preface_third'])) {
    $vars['attributes']['class'][] = 'preface-two';
  }
  else {
    $vars['attributes']['class'][] = 'preface-one';
  }

More efficient is a bit vague, especially as time and memory performance wouldnt differ enough to worry about unless you were iterating over this a great many times.

But with regards to readability, its largely about consistency and purpose clarity. Your writing style WILL change over time, sometimes in drastic ways but mostly a matter of verbosity and style.

There is nothing wrong with the above code per-say, but it is a bit 'cluttered', so readability can be improved by shortening. Exactly how far depends on you at this time, as consistency and personal readability are also key, especially for others or yourself having to read this code later on for any reason.

Im not saying this is the best way to do it, but personally I might try something like this to make it shorter and more efficient:

$checks = [!empty($vars['page']['preface_first']), !empty($vars['page']['preface_second']), !empty($vars['page']['preface_third'])];
$classes = [];

// other code

$vars['attributes']['class'] = array_merge($vars['attributes']['class'],$classes);

Then maybe one of the following flavours, largely depending on whether you expect this to grow more complicated and what seems more clear to you:

very explicit/verbose:

if(!empty($checks[0]) && !empty($checks[1]) && !empty($checks[2]))
{
    $classes[] = 'preface-three';
}
elseif(!empty($checks[0]) && !empty($checks[1]))
{
    $classes[] = 'preface-two';
}
elseif(!empty($checks[0]) && !empty($checks[2]))
{
    $classes[] = 'preface-two';
}
elseif(!empty($checks[1]) && !empty($checks[2]))
{
    $classes[] = 'preface-two';
}
else
{
    $classes[] = 'preface-one';
}

more compact:

if(!empty($checks[0]) && !empty($checks[1]) && !empty($checks[2]))
{
    $classes[] = 'preface-three';
}
elseif(!empty($checks[0]) && (!empty($checks[1]) || !empty($checks[2])))
{
    $classes[] = 'preface-two';
}
elseif(!empty($checks[1]) && !empty($checks[2]))
{
    $classes[] = 'preface-two';
}
else
{
    $classes[] = 'preface-one';
}

very compact:

if(!empty($checks[0]) && !empty($checks[1]) && !empty($checks[2]))
{
    $classes[] = 'preface-three';
}
elseif((!empty($checks[0]) && (!empty($checks[1]) || !empty($checks[2]))) || (!empty($checks[1]) && !empty($checks[2])))
{
    $classes[] = 'preface-two';
}
else
{
    $classes[] = 'preface-one';
}

There is also the ternary operator, which i do use frequently when there is one condition, but rarely when there are many.

http://php.net/manual/en/language.operators.comparison.php

$classes[] = (!empty($checks[0]) && !empty($checks[1]) && !empty($checks[2]))
            ? 'preface-three'
            : ((!empty($checks[0]) && (!empty($checks[1]) || !empty($checks[2]))) || (!empty($checks[1]) && !empty($checks[2])))
                ? 'preface-two'
                : 'preface-one';

As a side note, there is also something like the following, but i dislike it as it doesnt feel natural to me, so i stopped before finishing it

if(!empty($checks[0]))
{
    if(!empty($check[1]))
    {
        if(!empty($checks[2]))
        {
            $classes[] = 'preface-three';
        }
        else
        {
            $classes[] = 'preface-two';
        }
    }
}
elseif(!empty($checks[1])
{
    // ect. ect. - not recommended in my mind
}


How to approach separation of layers mapping response to model in Angular app? I want to organize mapping for my Angular app so that code structure would clear for other developers and I want to have as less code as possible.
I have an api service that is responsible for communicating with API. I create separate data services, which utilize the api service, to get backend data. The same data can be used in different features. I want to provide the data to some feature. But the feature needs a more complex model than backend provides. So, I want to do a complex mapping specific for the particular feature which extends the backend model. But I also do a basic mapping that is required for every feature like property naming ('_name' to 'name') or type converting (string to Date). What is your recommendation on how to approach this?
My current idea is the following. I put the data services in the app/core folder and do the basic mapping on this level. I have feature related models and mapper services in the app/feature folders. It looks OK in terms of separation of concerns. But I have doubts about the number of mapping functions and services. Looks wordy, doesn't it?
app/core/lesson
lesson-dto.model.ts
export interface LessonDto {
    id: string;
    _departmentId: string;
    begin: string;
    end: string;
}

get-lessons-response.model.ts
export interface GetLessonsResponse {
    lessons: LessonDto[];
}

lesson.model.ts
export interface Lesson {
    id: string;
    departmentId: string;
    begin: Date;
    end: Date;
}

lesson-basic-mapping.ts
export function mapLessonDtoToLesson(lessonDto: LessonDto): Lesson {
    return {
        id: lessonDto.id,
        departmentId: lessonDto._departmentId,
        begin: new Date(lessonDto.begin),
        end: new Date(lessonDto.end)
    };
}

lesson.data.service.ts
export class LessonDataService {

    constructor(private readonly apiService: ApiService) { }

    public getLessons(
        departmentId: string
    ): Observable<Lesson[]> {
        const requestUrl = `/departments/${departmentId}/lessons`;

        return this.apiService.get(requestUrl).pipe(
            map((response: GetLessonsResponse) => response.lessons
                .map(mapLessonDtoToLesson))
        );
    }
}

app/features/schedule
schedule-lesson.model.ts
export interface ScheduleLesson {
    id: string;
    name: string;
    begin: Date;
    end: Date;
    description: string;
}

scheduler-mapping.ts
export function mapLessonToScheduleLesson(lesson: Lesson): ScheduleLesson {
    return {
        id: lesson.id,
        name: calculateName(lesson.id, lesson.departmentId),
        begin: new Date(lessonDto.begin),
        end: new Date(lessonDto.end),
        description: doSomeOtherComplexMappingHere(lesson.departmentId)
    };
}

lesson-mapped-data.service.ts
export class LessonMappedDataService {

    constructor(private readonly lessonDataService: LessonDataService) { }

    public getLessons(
        departmentId: string
    ): Observable<ScheduleLesson[]> {
        return this.lessonDataService.get(departmentId).pipe(
            map((lessons: Lesson[]) => lessons
                .map(mapLessonToScheduleLesson))
        );
    }
}

One more thing that concerns me is that the more models I have or the longer they are the more similar code I have. I did some analysis. I like the approach with decorators. It looks similar to Newtonsoft in C# and hides the implementation of basic mapping. But it requires some hacky solutions to align it with TypeScript. And the feature of decorators is still unclear. So, I don't use it.
Do you have any ideas on how to make the code better (more clear and less to read)? And what do you think of the file/folder structure?
Thank you in advance.

I made some good experience with the following approach. Its more code, but it helped the other developers in the team (and especially the new ones) to get a good grip and a fast start.
As always, many roads, choose the one that fits best in your context, they all have drawbacks. :-)
First i describe the expected data from the Backend
export interface AddressJSON{
  street: string,
  city: string
};

export interface UserJSON{
  id: number,
  name: string,
  address: AddressJSON
}

That helps to understand what to expect from the backend. Also its easier to track mapping problems (for example a Java LONG that is stored in a JavaScript NUMBER => Number has a smaller value range)
Then i create the FE models and if the mapping between backend and FE is easy, then i add converter methods
class Address {
  public street: string;
  public city: string;

  constructor(street: string, city: string){
    this.street = street;
    this.city = city;
  }

  public toJSON(): AddressJSON{
    return {
      street: this.street,
      city: this.city
    }
  }

  public static fromJSON(address: AddressJSON): Address{
    return new Address(address.street, address.city);
  }
}

class User {
  public name: string;
  public address: Address;
  private readonly id: number;

  constructor(id:number){
    this.id = id;
  }

  public toJSON(): UserJSON {
    return {
      id: this.id,
      name: this.name,
      address: this.address.toJSON()
    }
  }

  public static fromJSON(user: UserJSON): User{
    const newUser: User = new User(user.id);
    newUser.name = user.name;
    newUser.address = Address.fromJSON(user.address);
    return newUser;
  }
}

As you can see the entities can be stacked.
One reason i choose classes over interfaces for my entities is that i can move functionality directly to the entities.
And, i can FORCE the way this entity has to be used. For example i can disallow the creation of a artificial user without an id.
It reduces the risc for later changes. And it enforces a deeper understanding of the business requirements behind those entities.
On the over side its much more work at the start.
Most people who didn´t liked this approach hated the extra work, that you have to invest. The people who liked it, mostly highlighted the better understanding of the business requirements and the possibly to enforce correct usage.


Sufficient implementation of subset sum for 45 min interview I am preparing for interviews, and have implemented a solution to this problem from Geeks for Geeks:

  Given a set of non-negative integers, and a value sum, determine if there is a subset of the given set with sum equal to given sum.

I don't fully have my head wrapped around the dynamic programming approach.  If I provided the implementation below in an interview, and explained that there was a dynamic solution that could speed the solution up for a small N and S would my technique be acceptable? What would your thoughts be as the interviewer?

FWIW I now that there would be duplicate processing at the leafs of my recursive descent. If asked I would say the complexity of my algorithm would be: \$\Theta(2^N + 1 + (N + 2(N-1) + ...))=\Theta(2^N)\$

Given a set of size 2: (1, 2) The calls would be (1, 2) -> ((1), (2))
Given a set of size 3: (1, 2, 3) The calls would be

                                 (1, 2, 3)
                         (1)(2, 3)       (1, 2)(3)
                            (2)(3)       (1)(2)

Looking at this pattern I would state there are \$2^N + 1\$ calls.

#include <iostream>
#include <iterator>
#include <vector>

void subset_sum(
    std::vector<int>::iterator begin, std::vector<int>::iterator end,
    const std::size_t sum, bool &found)
{
    std::size_t local_sum = 0;
    for (auto it = begin; it != end && !found; ++it) {
        local_sum += (*it);
    }

    if (sum == local_sum) {
        found = true;
    } else if (local_sum < sum) {
        found = false;
        return;
    }

    for (auto it = begin + 1; it < end && !found; ++it) {
        if (!found) {
          subset_sum(begin, it, sum, found);
        }
        if (!found) {
          subset_sum(it, end, sum, found);
        }
    }
}

int main()
{
    std::vector<int> set = {2, 3, 4};
    bool found = false;
    subset_sum(set.begin(), set.end(), 100, found);
    return (!found);
}

Bug
I ran your program, replacing 100 with 6.  This should have found the subset {2,4} as a solution but it didn't.
Your program searches for contiguous subsets which sum to the given value.  I don't think that contiguousnous was specified by the problem in the link you provided.


Handling errors in potentially incomplete responses I am using the library geoip2 to get Geolocation of many IP adderesses

"""
input:
        str:   IP
output ordered list:
    [0] str:   City, State, Country
    [1] tuple: (Lat; Log)
    [2] str:   Postal
"""

for i in pd.unique(df_to_print['requesterIp']):
    res = reader.city(i)
    # NOTE: Second snippet is added here
    myDict[i] = [res.city.names['en'] + ", " + res.subdivisions[0].names['en'] + ", " +  res.country.names['en'],(res.location.latitude, res.location.longitude), res.postal.code]

# output:  ['Calgary, Alberta, Canada', (50.9909, -113.9632), 'T2C']

Sometimes the response, which is in JSON, is missing some fields. This causes a exception.
Here is my proposed "fix", the code works as intended, but looks sinful
try:
    city = res.city.names['en']
except:
    city = "-1"

try:
    state = res.subdivisions[0].names['en']
except:
    state = "-1"

try:
    country =  res.country.names['en']
except:
    country =  "-1"

try:
    cord = (res.location.latitude, res.location.longitude)
except:
    cord = (-1, -1)

postal = res.postal.code if res.postal.code is not None else -1

print([city + ", " + state + ", " +  country, cord, postal])
# output: ['-1, -1, China', (34.7725, 113.7266), -1]

What can I do to make my code more professional and efficient?
(this will run for apx. 100K unique IPs, several times a hour; DB is local)

Some suggestions:

Don't use exceptions for flow control unless you have to for TOCTOU or other reasons. "Tell, don't ask" is a useful guideline when requesting something from a piece of state the current code has no control over, but it's not a law. In your code there is no chance of res.country.names changing while running res.country.names.get('en', "-1"), and that is much clearer than using exception handling to set a default.
You almost always want to catch specific exceptions.
When retrieving values from a dict you can use my_dict.get("key", default) to get a default value if the key does not exist.
Use f-strings rather than + to create formatted strings.


Django model for real estate I have built a real estate management system with Django. I want to know if my design database was wrong. Please let me know how to improve it.

Should I make the models use more than two tables (classes)?

class Profile(models.Model):
    user = models.OneToOneField(User, on_delete=models.CASCADE)
    name = models.CharField(max_length=50)
    image = models.ImageField(upload_to='profiles', default='logo.png')
    phone = models.CharField(max_length=11, default='')
    bio = models.CharField(max_length=100, default='')
    city = models.CharField(max_length=20, default='erbil')
    location = models.CharField(max_length=40, default='')
    date = models.DateTimeField(default=datetime.now)
    active = models.BooleanField(default=False)

    def __str__(self):
        return f'{self.user}'

class Listing(models.Model):
    objects = ListingManager()

    company = models.ForeignKey(User, on_delete=models.CASCADE )
    title = models.CharField(max_length=200)
    slug = models.SlugField(unique=True, default='', blank=True)
    address = models.CharField(max_length=200)
    city = models.CharField(max_length=100, choices=city_choices, default='lodon')
    estate_type = models.CharField(max_length=20, choices=estate_choices, default=house)
    description = models.TextField(blank=True)
    rent_sale = models.CharField(max_length=20, choices=rent_sale_choice, default=sale)
    price = models.IntegerField()
    bedrooms = models.IntegerField(default=0)
    bathrooms = models.DecimalField(max_digits=2, decimal_places=1)
    garage = models.IntegerField(default=0)
    sqft = models.IntegerField()
    sold =  models.BooleanField(default=False)
    sold_time = models.DateTimeField(default=datetime.now,blank=True)
    photo_main = models.ImageField(upload_to='listings_main')
    photo_1 = models.ImageField(upload_to='listings_1', blank=True)
    photo_2 = models.ImageField(upload_to='listings_1', blank=True)
    photo_3 = models.ImageField(upload_to='listings_1', blank=True)
    photo_4 = models.ImageField(upload_to='listings_1', blank=True)
    photo_5 = models.ImageField(upload_to='listings_1', blank=True)
    photo_6 = models.ImageField(upload_to='listings_1', blank=True)
    is_published = models.BooleanField(default=True)
    list_date = models.DateTimeField(default=datetime.now, blank=True)
    unless = models.CharField(default='', max_length=20)

As a rule of thumb in programming, "there are only four numbers": zero, one, two, and "many".  You have seven photo fields, which means that you should just treat that as "many".  Not only is that an unfortunate artificial limitation (someday there will be a listing with eight photos, and you won't be able to accommodate it easily), it's also a pain to write code that uses your schema (you either have to copy-and-paste code six times, or use variably-named identifiers).

What you want is a separate table for photos, to support a one-to-many relationship.


HeapSort implementation I have implemented the heap sort algorithm which passes my tests. Although, I would like to ask you if I can improve anything from readability to performance.

import java.util.List;

/**
 * Created by Orestis on 17/07/2015
 */

public class HeapSort {

    public static <T extends Comparable<? super T>> void sort(List<T> list) {
        heapSort(list);
    }

    private static <T extends Comparable<? super T>> void heapSort(List<T> list){
        int heapSize = list.size() - 1;
        buildMaxHeap(list, heapSize);
        for (int i = list.size() - 1; i >= 1; i--) {
            exchange(0, i, list);
            heapSize--;
            maxHeapify(list, 0, heapSize);
        }
    }

    /**
     *
     * Maintain the max-heap property (list[find_parent(i)] >= list[i])
     * If the max-heap property is violated float down list[i]
     */
    private static <T extends Comparable<? super T>> void maxHeapify(List<T> list, int index, int heapSize) {
        int largest = index; // initialise largest to index.

        int left = findLeftIndex(index);
        int right = findRightIndex(index);

        if (left <= heapSize && list.get(left).compareTo(list.get(index)) >= 1) {
            largest = left;
        }

        if (right <= heapSize && list.get(right).compareTo(list.get(largest)) >= 1) {
            largest = right;
        }

        if (largest != index) {
            exchange(index, largest, list);
            maxHeapify(list, largest, heapSize);
        }
    }

    /**
     *
     * This function goes through the remaining nodes of the heap tree and
     * runs maxHeapify on each one.
     */
    private static <T extends Comparable<? super T>> void buildMaxHeap(List<T> list, int heapSize) {
        int start = (int) Math.floor((heapSize / 2));
        for (int i = start; i >= 0; i--) {
            maxHeapify(list, i, heapSize);
        }
    }

    private static <T extends Comparable<? super T>> void exchange(int i, int largest, List<T> array){
        T temp = array.get(largest);
        array.set(largest, array.get(i));
        array.set(i, temp);
    }

    private static int findParentIndex(int index) {
        return (index >> 1) ^ 1;
    }

    private static int findLeftIndex(int index) {
        return (index << 1) ^ 1;
    }

    private static int findRightIndex(int index) {
        return (index << 1) + 2;
    }
}

What comes to performance, it will degrade drastically if you pass a LinkedList (which implements java.util.List interface accepted in your heap sort): for such a list, get(int) operation runs not in constant time, but rather \$O(n)\$. In order to deal with it, check in your sort that the input list is not a linked list, and if it is, copy all the elements to an ArrayList, sort it, and load back to the input list.

The second point concerns the sifting algorithm. If the indices of the sifting operations are \$i_1, i_2, ..., i_n\$, you first swap \$i_1\$ with \$i_2\$, then \$i_2\$ with \$i_3\$ and so on until the element is in its proper place \$i_n\$. You can do better: save the element at \$i_1\$, then move \$i_2\$ to \$i_1\$, \$i_3\$ to \$i_2\$, and so on. Finally, just set the saved element at index \$i_n\$. This optimization does not improve the time complexity of heap sort, yet it reduces the constant factors hidden by the big-O notation.

The last, but not least: sometimes we do not need to sort the entire array/list. For this, it makes sense to code up a sorting routine, which can sort particular ranges. Then, in order to sort the entire array/list - as you do - you can just do:

sort(List<T> list) {
    sort(list, 0, list.size());
}


Project Euler # 55 Lychrel numbers in Python If we take 47, reverse and add, 47 + 74 = 121, which is palindromic.

Not all numbers produce palindromes so quickly. For example,

349 + 943 = 1292,
1292 + 2921 = 4213
4213 + 3124 = 7337

That is, 349 took three iterations to arrive at a palindrome.

Although no one has proved it yet, it is thought that some numbers, like 196, never produce a palindrome.
A number that never forms a palindrome through the reverse and add process is called a Lychrel number.
Due to the theoretical nature of these numbers, and for the purpose of this problem, we shall assume
that a number is Lychrel until proven otherwise. In addition you are given that for every number below
ten-thousand, it will either (i) become a palindrome in less than fifty iterations, or, (ii) no one,
with all the computing power that exists, has managed so far to map it to a palindrome. In fact, 10677
is the first number to be shown to require over fifty iterations before producing a palindrome:
4668731596684224866951378664 (53 iterations, 28-digits).

Surprisingly, there are palindromic numbers that are themselves Lychrel numbers; the first example is 4994.

How many Lychrel numbers are there below ten-thousand?

from time import time

def is_palindrome(number):
    """returns True for a palindrome, False otherwise."""
    to_str = str(number)
    return to_str == to_str[::-1]

def add_reverses(number, count):
    """makes a list of reverses for a number at a maximum iteration of count, breaks if palindrome found below count."""
    reverses = [number]
    for _ in range(count):
        last_number = reverses[-1]
        new_number = int(str(reverses[-1])[::-1])
        if is_palindrome(last_number + new_number):
            reverses.append(last_number + new_number)
            return reverses
        else:
            reverses.append(last_number + new_number)
    return reverses

def count_lychrel_range(number_range, count):
    """returns Lychrel numbers within number_range, assumes count is the maximum iterations for a number."""
    total = 0
    numbers_reverses = {}
    for number in range(number_range):
        numbers_reverses[number] = add_reverses(number, count)
    for number, rev_sequence in numbers_reverses.items():
        if len(rev_sequence) >= count:
            total += 1
    return total

if __name__ == '__main__':
    start_time = time()
    n = 10000
    print(f'Total Lychrel numbers below {n}: {count_lychrel_range(n, 50)}')
    print(f'Time: {time() - start_time} seconds.')

I like the functions you've divided up work. Good job.

def is_palindrome(number):
    """returns True for a palindrome, False otherwise."""

Its a little nickpicky, but the pep on docstrings recommends

  The docstring is a phrase ending in a period. It prescribes the
  function or method's effect as a command ("Do this", "Return that"),
  not as a description; e.g. don't write "Returns the pathname ...".

I would change the docs here to something more like

"""Return whether a number is the same if read backwards and forwards."""

last_number = reverses[-1]
new_number = int(str(reverses[-1])[::-1])

Since last_number is reverses[-1], use it. Saves a lookup.

last_number = reverses[-1]
reversed_number = int(str(last_number[::-1]))

if is_palindrome(last_number + new_number):
    reverses.append(last_number + new_number)
    return reverses
else:
    reverses.append(last_number + new_number)

Since you have the same code in each path, and nothing before it, you can hoist it up out of the if statement

reverses.append(last_number + new_number)
if is_palindrome(last_number + new_number):
    return reverses

for _ in range(count):
    ...
    if ...:
        return reverses
    ...
return reverses

+1 for using _ as the loop variable.

This is again nitpicky, but since you have an opportunity to make this code only have one return point, why not take it?

for _ in range(count):
    last_number = reverses[-1]
    new_number = int(str(reverses[-1])[::-1])

    reverses.append(last_number + new_number)
    if is_palindrome(last_number + new_number):
        break

return reverses

def count_lychrel_range(number_range, count):
    """returns Lychrel numbers within number_range, assumes count is the maximum iterations for a number."""

Except it takes the upper bound of the range, not a number range. The doc clears it up, but just looking at the method signature I would assume you pass in the list of numbers to iterate over or a range. Since the expected call is not

count_lychrel_range(range(500, 1000), 50)

I would suggest changing the names here

def count_lychrel_numbers(upper_bound, max_iterations):

Since you only care about the length of the list, why go through all the effort of building a list of intermediate numbers from the process? In fact, you only need the length so you can compare it to the maximum number of iterations to run. Doing the test means we can just return a yes/no answer, rather than an entire list of numbers.

I think something like this inplace of add_reversed would be a little nicer.

def is_lychrel(number, max_iterations):
    """Return whether a number is a lychrel number.
    A lychrel number is one which becomes a palindrome when it is repeatedly reversed and added to itself."""
    for _ in range(max_iterations):
        reversed_number = int(str(number)[::-1])
        number += reversed_number
        if is_palindrome(number):
            return True
    return False

Then count_lychrel_numbers can be a simple loop

def count_lychrel_numbers(upper_bound, max_iterations):
    """..."""
    total = 0
    for number in range(upper_bound):
        if is_lychrel(number, max_iterations):
            total += 1
    return total

Or if you want to use sum

def count_lychrel_numbers(upper_bound, max_iterations):
    """..."""
    return sum(1 for number in range(upper_bound) if is_lychrel(number, max_iterations))

As a bonus, you can see that "new_number" is reversed twice. Once in is_palindrome and once in add_reverses/is_lychrel. Can you reduce it down to just one reverse without bloating the code?


Another Sliding Window - find each `k-size` window the maximum num. from an integer array/list I've searched this site and find many similar questions, but not exactly the same as the one I'll post.
This attempts to solve Leetcode 239:

You are given an array of integers nums, there is a sliding window of size k which is moving from the very left of the array to the very right. You can only see the k numbers in the window. Each time the sliding window moves right by one position.
Return the max sliding window.

The subject is - given an integer array, and positive integer k, denoting the sliding window size. Each time you can only slide from left to right one position, try to find each windows maximum number and output these maximum number into an output list. [Note - the given list could be very large ~ 1 Million (nums)]
For example:  Given a nums array =  [99, 74, 48, 27, 56, 88, 66, 77, 101]; k = 3
Output should be:     [99, 74, 56, 88, 88, 88, 101]
I've tried to solve this in Python, and wonder if it could be further optimized and improved. Thanks in advance.
def maxSliding(nums: List[int], k: int) -> List[int]:
    if not k or not nums: return []

    z = len(nums)

    if z <= k:  return [max(nums, default = 0)]

    q = deque([])
    res = []

    for i, num in enumerate(nums):
        if q and q[0] == i-k:
            q.popleft()

        while q and nums[q[-1]] < num:
            q.pop()

        q.append(i)
        if i >= k-1:  res.append(nums[q[0]])
    return res

# Since every num. enters and leaves the queue at most once, the time
# complexity is O(len(A)), independent of the value of K.
if __name__ == '__main__':
    B = [99,  74, 48, 27, 56, 88, 66, 77, 101]
    print(maxSliding(B, 3))

I don't see any obvious improvements to your algorithm.
Tests:
Your "main" section seems to be checking if the algorithm works, but you can make better use of the examples provided with the original prompt.
Constraints:
The original prompt makes certain promises about the inputs. It's reasonable to just take those as assumptions, or (maybe better, depending on stuff) to make them explicit assertions, but there's no need to handle violating cases.
Typing:
You've got type-hints in your function signature.
That's great!
But in python the idea of "types are comments that can't be wrong" only holds if you actually use a type-checker. Trying mypy on your code gives an error that it can't figure out the type of q. The easiest fix for this would be to use the Deque from typing (or upgrade to python 3.9) so you can write q = Deque[int]().
Begging for microseconds:
You can save yourself some conditional checks by changing how you handle the first couple rounds: Start the loop with something in the deque, and slice off the head of the result-list at the end. On the other hand, trying to optimize at that level of granularity is easy to get wrong. I banged out an over-engineered version, but without benchmarking this is worthless.
from typing import List, Deque, Iterable, Tuple, Callable
from itertools import islice

def _window_func(k: int, q: Deque[Tuple[int, int]]) -> Callable[[Tuple[int, int]], int]:
    def retval(pair):
        if q[0][0] == pair[0] - k:
            q.popleft()
        while q and q[-1][1] < pair[1]:
            q.pop()
        q.append(pair)
        return q[0][1]
    return retval

def maxSliding(nums: List[int], k: int) -> List[int]:
    assert nums
    assert 1 <= k <= len(nums)
    return list(islice(
        map(_window_func(k, Deque([(-1, nums[-1])])),
            enumerate(nums)),
        k - 1, None
    ))

tests = [([99,74,48,27,56,88,66,77,101], 3, [99,74,56,88,88,88,101]),
         ([1,3,-1,-3,5,3,6,7], 3, [3,3,5,5,6,7]),
         ([1], 1, [1]),
         ([1,-1], 1, [1,-1]),
         ([9,11], 2, [11]),
         ([4,-2], 2, [4])]

if __name__ == '__main__':
    for (l, k, result) in tests:
        try:
            actual = maxSliding(l, k)
            assert actual == result
        except:
            print(f'Offending test {(l, k, result)}, got {actual}')
            raise
    print("tests passed")
```


Counting Valleys - HackerRank Challenge For this challenge on HackerRank:

  A valley is a sequence of consecutive steps below sea level, starting with a step down from sea level and ending with a step up to sea level.

  Given Gary's sequence of up and down steps during his last hike, find and print the number of valleys he walked through.

  For example, if Gary's path is s=[DDUUUUDD], he first enters a valley 2 units deep. Then he climbs out and up into a mountain 2 units high. Finally, he returns to sea level and ends the hike.

I reached a solution, but I'm sure it can be optimized:

function countingValleys(n, s) {
    let heightTracker = [];
    let height = 0;
    for (let i = 0; i < s.length; i++) {
        if (s[i] === "D") {
            height--;
            heightTracker.push(height);
        }
        if (s[i] === "U") {
            height++;
            heightTracker.push(height);
        }
    }
    let planeTracker = [];
    for (let j = 0; j < heightTracker.length; j++) {
        if (heightTracker[j] < 0) {
            planeTracker.push("valley");
        }
        if (heightTracker[j] > 0) {
            planeTracker.push("mountain");
        }
        if (heightTracker[j] === 0) {
            planeTracker.push("flat");
        }
    }
    let newArray = [];
    for (let k = 0; k < planeTracker.length; k++) {
        if (planeTracker[k] === planeTracker[k - 1]) {
            continue;
        }
        if (planeTracker[k] !== planeTracker[k - 1]) {
            newArray.push(planeTracker[k]);
        }
    }
    let valleyCount = 0;
    for (let l = 0; l < newArray.length; l++) {
        if (newArray[l] === "valley") {
            valleyCount++;
        }
    }
    return valleyCount;
}

My logic was to keep track of all negative and positive values in s.

If heightTracker[j] is negative, push "valley" into planeTracker
If heightTracker[j] is positive, push "mountain" into planeTracker
Otherwise, push "flat" into planeTracker

So at that point, I'm left with:

[ 'valley', 'valley', 'valley', 'flat', 'valley', 'valley', 'valley', 'valley', 'valley', 'flat', 'mountain', 'flat' ]

And I'm wondering, is there a way to filter out any elements that are the same as the element before them? The goal was to generate the array:

[ 'valley', 'flat', 'valley', 'flat', 'mountain', 'flat'] and then count the number of times the word "valley" appears in that array. As you can see, I did this with a for-loop. But I'm wondering what other suggestions you'd have - maybe the .filter() method?

You're vastly overcomplicating the solution by introducing three arrays heightTracker, planeTracker, and newArray.  None of those is needed: just track the elevation (which is a better name than height), and go by the definition:

  A valley is a sequence of consecutive steps below sea level, starting with a step down from sea level and ending with a step up to sea level.

So, the number of valleys traversed is the number of times Gary's elevation changes from -1 to 0.

function countingValleys(n, s) {
    let elevation = 0;
    let traversedValleys = 0;
    for (let i = 0; i < n; i++) {
        if (s[i] === "D") {
            --elevation;
        } else if (s[i] === "U") {
            if (++elevation === 0) traversedValleys++;
        }
    }
    return traversedValleys;
}


Pomodoro Timer via setTimeout() I've made my attempt to code timer entirely by hand using jQuery. No external libraries to manage time at all.

$(document).ready(function() {
 var setSessionMinutes = 5,
  setBreakMinutes = 5,
  sessionMinutes = setSessionMinutes,
  breakMinutes = setBreakMinutes,
  counterSeconds = -1,
  countTimeout,
  runState = false;

 function setClockValues() {
  $('#break').html(setBreakMinutes);
  $('#session').html(setSessionMinutes);
  $('#time').html(formatTime(setSessionMinutes, 0));
  sessionMinutes = setSessionMinutes;
  breakMinutes = setBreakMinutes;
  counterSeconds = -1;
 };
 setClockValues();

 function formatTime(minutes, seconds) {
  minutes = minutes >= 10 ? minutes : '0' + minutes;
  seconds = seconds >= 10 ? seconds : '0' + seconds;
  return minutes + ':' + seconds;
 }

 //change values by buttons
 $('#minusbreak').click(function() {
  $('#break').html(setBreakMinutes > 0 ? --setBreakMinutes : 0);
  breakMinutes = setBreakMinutes;
 });
 $('#plusbreak').click(function() {
  $('#break').html(++setBreakMinutes);
  breakMinutes = setBreakMinutes;
 });
 $('#minusSession').click(function() {
  $('#session').html(setSessionMinutes > 0 ? --setSessionMinutes : 0);
  $('#time').html(formatTime(setSessionMinutes, 0));
  sessionMinutes = setSessionMinutes;
 });
 $('#plusSession').click(function() {
  $('#session').html(++setSessionMinutes);
  $('#time').html(formatTime(setSessionMinutes, 0));
  sessionMinutes = setSessionMinutes;
 });

 function countDown() {
  if (counterSeconds < 0 && sessionMinutes >= 0) {
   --sessionMinutes;
   counterSeconds = 59;
  }
  $('#time').html(formatTime(sessionMinutes, counterSeconds));
  --counterSeconds;
  if (counterSeconds >= 0 && sessionMinutes >= 0)
   countTimeout = setTimeout(countDown, 1000);
  else {
   $('#finish').get(0).play();
   setTimeout(countDownBreak, 1000);
  }
 }

 function countDownBreak() {
  if (counterSeconds < 0 && breakMinutes >= 0) {
   --breakMinutes;
   counterSeconds = 59;
  }
  $('#time').html(formatTime(breakMinutes, counterSeconds));
  --counterSeconds;
  if (counterSeconds >= 0 && breakMinutes >= 0)
   countTimeout = setTimeout(countDownBreak, 1000);
  else {
   stopCountDown();
   $('#status').html('start');
   runState = false;
   $('#finish').get(0).play();
  }
 }
 function stopCountDown() {
  clearTimeout(countTimeout);
  setClockValues();
 }

 //   start/stop timer
 $('#status').click(function() {
  switch (runState) {
   case false:
    $('#status').html('reset');
    countDown();
    runState = true;
    break;
   case true:
    $('#status').html('start');
    stopCountDown();
    runState = false;
    break;
  }
 });

});

You can also see it on CodePen.io.

I realise this code should be 3/4 or even half current length, more DRY etc., but I'd like to get some opinions about code without much optimization to know better where I currently am with my progress. After all, I could spend 12 hours to make it beautiful (which I eventually will), but that would destroy purpose of this post.

About setTimeout instead of setInterval: I did it on purpose because of this.

Feedback

I know it has been nearly 3 years since this was posted and you likely have learned more about javascript and jQuery since then. Perhaps the information below is old news but nevertheless...

I like the styles and the sound that plays when the timer is done is fun! I did notice that if I decrease the Session Length to 0 and click the start button, it shows -01:59 and then appears to jump up to 05:58. Perhaps it would be wise to not allow the user to click the start button if the value for session length is 0.

Recommendations

I only see a couple things I would change:

It is recommended that DOM elements be stored in a variable - e.g.

var breakElement = $('#break');

And then later on use breakElement instead of $('#break') - e.g.

breakElement.html(setBreakMinutes);

The switch statement at the end seems a bit heavy - why not just use an if/else:

switch (runState) {
        case false:
            $('#status').html('reset');
            countDown();
            runState = true;
            break;
        case true:
            $('#status').html('start');
            stopCountDown();
            runState = false;
            break;
    }

While it is only few lines shorter, there is no need to remember the break statements:

if (runState) {
    $('#status').html('start');
    stopCountDown();
    runState = false;
}
else {
    $('#status').html('reset');
    countDown();
    runState = true;
}


Simultaneously searching listings by attributes such as category, title, postcode Is the code below safe from SQL injection?

<?

mysql_connect ("localhost", "db","pass")  or die (mysql_error());
mysql_select_db ("db");

$term = mysql_real_escape_string($_POST['term']);

$sql = mysql_query("select * from tblListings where category like '%$term%' or title like '%$term%' or postcode like '%$term%' or info like '%$term%' ");

function highlight($needle, $haystack)
{
    return preg_replace('/(' . preg_quote($needle, '/') . ')/i', '$1', $haystack);
}

while ($row = mysql_fetch_array($sql)){

    echo '
 Category: ' . highlight($term, $row['category']);
    echo '
 Title: ' . highlight($term, $row['title']);
    echo '
 Address: ' . highlight($term, $row['add1']);
    echo '
 Street: ' . highlight($term, $row['street']);
    echo '
 City: ' . highlight($term, $row['city']);
    echo '
 Postcode: ' . highlight($term, $row['postcode']);
    echo '
 Phone: ' . highlight($term, $row['phone']);
    echo '
 E-Mail: ' . highlight($term, $row['email']);
    echo '
 Website: ' . highlight($term, $row['website']);
    echo '
 Info: ' . highlight($term, $row['info']);
    echo '

';
    //echo '
 E-Mail:   '.$row['email']; use this for the fields that you dont want to search

}

?>

I HIGHLY suggest using PHP PDO with prepared statements ALONG with filtering each field for specific accepted characters.

But to answer your question, your method above is no longer considered safe and can be exploited.

PDO & Tutorial:

http://php.net/manual/en/book.pdo.php
http://net.tutsplus.com/tutorials/php/why-you-should-be-using-phps-pdo-for-database-access/

You can use preg_match to filter for specific character sequences.

Basic Regex Filter Example:

if (preg_match('/^[0-9-]+$/', $_POST['postcode']))
{
    //POSTAL CODE IS GOOD TO GO
    //Filters for charaters "0-9" along with "-" for longer postal codes
}
else
{
    //REJECT
}


Hide link formatting Firefox extension I'm building a Mozilla Firefox extension to hide link's formatting and show them as regular text. It consists of a script that runs on web-pages (content.js) that's activated by clicking on the extension (handled with background.js). On activation, a CSS rule is injected to hide link's formatting. Activating it again will shut changes off.
background.js additionally updates the extension's icon (based on links being currently hidden or not) every time the active tab is changed.
I'm new to JavaScript. I'd really appreciate your comments and observations to improve it (best practices, code structure, etc).
Script that runs on web pages (content.js):
'use strict';

// Links formatting state variable
let hideLinks = false;
const hideLinksRule = `
a,
a:hover,
a:focus,
a:active,
a:visited {
    text-decoration: none !important;
    color: inherit !important;
    background-color: inherit !important;
    border-bottom: initial !important;
}`;

let styleSheet = (function () {
    let style = document.createElement('style');
    document.head.appendChild(style);
    return style.sheet;
})();

function handleRequest(request) {

    // Do not modify if request is only a query
    if (request.isQuery) {
        return Promise.resolve({
            hideLinks
        });
    }

    // Toggle hide/show link formatting
    if (hideLinks) {
        styleSheet.deleteRule(0);
        hideLinks = !hideLinks;
    } else {
        styleSheet.insertRule(hideLinksRule, 0);
        hideLinks = !hideLinks;
    }

    // Return links state
    return Promise.resolve({
        hideLinks
    });

}

browser.runtime.onMessage.addListener(handleRequest);

Script to handle the extension on the browser (background.js):
// Icons paths
const showLinksIconPath = {
    path: 'icons/link-icon.png'
};
const hideLinksIconPath = {
    path: 'icons/broken-link-icon.png'
};

function onError(error) {
    browser.browserAction.setIcon(showLinksIconPath);
}

function updateIcon(response) {
    if (response.hideLinks) {
        browser.browserAction.setIcon(hideLinksIconPath);
    } else {
        browser.browserAction.setIcon(showLinksIconPath);
    }
}

function verifyTabStatus(activeInfo) {
    browser.tabs.sendMessage(
        activeInfo.tabId, {
            isQuery: true
        }
    ).then(updateIcon).catch(onError);
}

function activeTabLinkToggle() {
    browser.tabs.query({
        currentWindow: true,
        active: true
    }).then(requestLinkFormatToggle).catch(onError);
}

function requestLinkFormatToggle(tabs) {

    if (!(tabs === undefined || tabs.length == 0)) {
        const activeTab = tabs[0];
        browser.tabs.sendMessage(
            activeTab.id, {
                isQuery: false
            }
        ).then(updateIcon).catch(onError);
    }

}

// Update Icon on tab change
browser.tabs.onActivated.addListener(verifyTabStatus);

// Request toggle link formatting on click
browser.browserAction.onClicked.addListener(activeTabLinkToggle);

Configuration file to put it together (manifest.json):
{

    "manifest_version": 2,
    "name": "No-link",
    "version": "1.0",

    "description": "Hide link formatting.",

    "icons": {
        "48": "icons/link-icon.png"
    },

    "permissions": [
        "tabs"
    ],

    "browser_action": {
        "default_icon": "icons/link-icon.png",
        "default_title": "No-link"
    },

    "background": {
        "scripts": ["background.js"]
    },

    "content_scripts": [
        {
            "matches": ["<all_urls>"],
            "js": ["content.js"]
        }
    ]

}

You can also get the source code here.
Thank you very much for your time.

Always prefer const when declaring variables - only use let when you have to reassign a variable. const requires less cognitive overhead, when you know at a glance that something's never going to be reassigned.
Style toggling You're currently toggling the style with styleSheet.insertRule(rule, 0) and styleSheet.deleteRule(0). I think it would be a little bit better to take a different approach.

Who knows what insertRule(rule, 0) and deleteRule(0) do at a glance? Probably not a whole lot of people; it's not a very common API that's used.
What if you need to expand the extension to add additional rules? Then using multiple rule indicies could become repetitive.

One alternative would be to, on toggle, either append or remove the style tag from the <head>. Another alternative would be to assign or clear its textContent. See below for example.
hideLinks is not an altogether precise name. At first glance, someone seeing a variable named hideLinks would probably think it sounds like a function that, when called, hides links. Maybe call it something else, like linksAreHidden. You can also make its toggling more DRY:
if (hideLinks) {
    styleSheet.deleteRule(0);
    hideLinks = !hideLinks;
} else {
    styleSheet.insertRule(hideLinksRule, 0);
    hideLinks = !hideLinks;
}

can be
if (hideLinks) {
    styleSheet.deleteRule(0);
} else {
    styleSheet.insertRule(hideLinksRule, 0);
}
hideLinks = !hideLinks;

Or, even better, rather than having a persistent outside variable, you could simply check to see whether the style element is attached or has content. See below for example. This is what I'd prefer - let the style node (which must exist regardless) be the source of truth, rather than introducing another variable that needs to stay in sync.
Request object You currently have
if (request.isQuery) {
    return Promise.resolve({
        hideLinks
    });
}
// otherwise, insert or remove style rule

I think it'd be better to be more explicit. What if you add a 3rd request command, or a 4th request command? Instead of having an implicit "Else, if this isn't a query, then do THIS instead", I'd prefer to always send a property indicating what the command should be.
Perhaps like this, using a isToggle property:
const hideLinksStyleText = `
a,
a:hover,
a:focus,
a:active,
a:visited {
text-decoration: none !important;
color: inherit !important;
background-color: inherit !important;
border-bottom: initial !important;
}`;
const styleTag = document.head.appendChild(document.createElement('style'));

function handleRequest(request) {
    const linksOriginallyHidden = Boolean(styleTag.textContent);
    // Do not modify if request is only a query
    if (request.isQuery) {
        return Promise.resolve({
            linksAreHidden: linksOriginallyHidden
        });
    }
    if (request.isToggle) {
        styleTag.textContent = linksOriginallyHidden
            ? hideLinksStyleText
            : '';
        return Promise.resolve({
            linksAreHidden: !linksOriginallyHidden
        });
    }
}

The conditional operator can be used to make conditional expressions tersely. This:
const showLinksIconPath = {
    path: 'icons/link-icon.png'
};
const hideLinksIconPath = {
    path: 'icons/broken-link-icon.png'
};
function updateIcon(response) {
    if (response.hideLinks) {
        browser.browserAction.setIcon(hideLinksIconPath);
    } else {
        browser.browserAction.setIcon(showLinksIconPath);
    }
}

can turn into
function updateIcon(response) {
    browser.browserAction.setIcon({
        path: `icons/${response.hideLinks ? 'broken-' : ''}link-icon.png`
    });
}

Or, if you don't like the interpolation:
function updateIcon(response) {
    browser.browserAction.setIcon({
        path: response.hideLinks
            ? 'icons/broken-link-icon.png'
            : 'icons/link-icon.png'
    });
}

Tab checking You have:
if (!(tabs === undefined || tabs.length == 0)) {
  const activeTab = tabs[0];

The negations are slightly tougher to parse logically at a glance than they can be. Consider instead:
if (tabs && tabs.length) {

In a larger project, you could use optional chaining and Babel:
if (tabs?.length) {

But optional chaining is only supported in very new versions of FF, and Babel isn't worth it just for this. (Babel is a good idea if you want to write a reasonably sized script and want to permit older browser versions to be able to understand the JS syntax you use. For a FF extension, it's less important, because almost all users will be on reasonably up-to-date browsers anyway - no need to transpile down to ES5 for those crummy IE11 users)


Finding the nth Fibonacci number using recursive technique EDIT

So I've been working on this according to the challenge @PeterTaylor gave me. I'm not very good at math, so I've been doing a lot of reading and found this link on the Binet form. Fibonacci algorithms

Unfortunately, when I use this in Ruby, it typically returns Infinity above 1000 or so for n. It works well for smaller equations, but not so much for very large numbers.

Here is the code I wrote for it.

a = (1 + Math.sqrt(5))/2

b = (1 - Math.sqrt(5))/2
#here, n == 5
f = ((a**5) - (b**5))/(a-b)

When you round f.to_i, it returns 5.

Back to the drawing board.

Original post below

Practicing recursion some more, and I'm wondering if this is the proper solution for this exercise I'm doing.

First, my code in Ruby:

def fibonacci(num, fact=[1,1])
  x = fact[fact.length-1]
  y = fact[fact.length-2]
  if fact[num-1]
    return fact[num-1]
  else
    fibonacci(num, fact.push(x+y))
  end
end

p fibonacci(5)
p fibonacci(12) == 144
p fibonacci(20) == 6765
p fibonacci(8200)

The exercise is this: Write a recursive method that computes the nth Fibonacci number, where nth is an argument to the method.

So, the 12th fibonacci number (which would be fact[11] due to indexing) is 144.

I know that I am returning the correct number, and that the recursion stops once fact[num-1] returns true. I can't help but feel that this is not the best way to do this, or even the proper way to do this.

Thoughts? Any advice?

Firstly, a minor point. fact? Have you refactored a recursive factorial method and forgotten to rename the accumulator?

That aside, I find this hilarious. Almost certainly the intended solution was along the lines of

def fibonacci(num)
  if num < 2
    return 1
  else
    return fibonacci(num - 2) + fibonacci(num - 1)
  end
end

which is the naïve recursive definition, and a classic teaching example of how naïve implementations can be insanely inefficient.

Your answer meets the rather vague spec (it is recursive), but is much better than the intended solution because it takes linear time (or maybe quadratic if push is linear, but I doubt it) rather than exponential.

If you want an additional challenge, here are two:

Your solution takes linear time and linear space. Can you modify it to take linear time and constant space?
Can you modify it further to take logarithmic time and constant space?

Hint for the second one:

   \$F(m+n) = F(m+1) F(n) + F(m) F(n-1)\$


RegExp selecting strings surrounded by brackets My purpouse is to select every character which is surrounded by { and }, this is easily achievable using this regexp {\w*}.

I've developed an extenstion method for strings:

    public static IEnumerable<string> ObtainTokens(this string originalString)
    {
        Regex expression = new Regex(@"{\w*}");
        foreach (Match element in expression.Matches(originalString))
        {
            //Exclude the brackets from the returned valued
            yield return Regex.Replace(element.Value, @"{*}*", "");
        }
    }

Is there a way to get rid of of Regex.Replace?
Returning the values as IEnumerable is a good choice?

Modify your regexp: {(\w*)} and then replace:

yield return Regex.Replace(element.Value, @"{*}*", "");

with

yield return element.Groups[1].Value;

ps: full code is avaialbe here